From: "Liam R. Howlett" <Liam.Howlett@oracle.com>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: oe-kbuild@lists.linux.dev, Xiao Yang <ice_yangxiao@163.com>,
linux-mm@kvack.org, lkp@intel.com, oe-kbuild-all@lists.linux.dev,
linux-kernel@vger.kernel.org, ltp@lists.linux.it,
oliver.sang@intel.com
Subject: Re: [PATCH] mm/vma: Return the exact errno for __split_vma() and mas_store_gfp()
Date: Tue, 10 Sep 2024 09:55:08 -0400 [thread overview]
Message-ID: <bfwh67n7cztablduizthnipozy7fagsrsb33vqawv3s3bh53vo@ygxo2rzsomlc> (raw)
In-Reply-To: <d30bf8ee-6edc-4bea-adba-8d54ba520b6c@stanley.mountain>
* Dan Carpenter <dan.carpenter@linaro.org> [240910 09:37]:
> Hi Xiao,
>
> kernel test robot noticed the following build warnings:
>
> url: https://github.com/intel-lab-lkp/linux/commits/Xiao-Yang/mm-vma-Return-the-exact-errno-for-__split_vma-and-mas_store_gfp/20240909-130325
> base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
> patch link: https://lore.kernel.org/r/20240909050226.2053-1-ice_yangxiao%40163.com
> patch subject: [PATCH] mm/vma: Return the exact errno for __split_vma() and mas_store_gfp()
> config: x86_64-randconfig-161-20240910 (https://download.01.org/0day-ci/archive/20240910/202409102026.LOh8J1Lh-lkp@intel.com/config)
> compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202409102026.LOh8J1Lh-lkp@intel.com/
>
> smatch warnings:
> mm/vma.c:1263 vms_gather_munmap_vmas() warn: missing error code 'error'
>
> vim +/error +1263 mm/vma.c
>
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1166 /*
> dba14840905f9e Liam R. Howlett 2024-08-30 1167 * vms_gather_munmap_vmas() - Put all VMAs within a range into a maple tree
> 6898c9039bc8e3 Liam R. Howlett 2024-08-30 1168 * for removal at a later date. Handles splitting first and last if necessary
> 6898c9039bc8e3 Liam R. Howlett 2024-08-30 1169 * and marking the vmas as isolated.
> 6898c9039bc8e3 Liam R. Howlett 2024-08-30 1170 *
> dba14840905f9e Liam R. Howlett 2024-08-30 1171 * @vms: The vma munmap struct
> 6898c9039bc8e3 Liam R. Howlett 2024-08-30 1172 * @mas_detach: The maple state tracking the detached tree
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1173 *
> 6898c9039bc8e3 Liam R. Howlett 2024-08-30 1174 * Return: 0 on success, -EPERM on mseal vmas, -ENOMEM otherwise
>
> This comment needs to be updated.
>
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1175 */
> 9014b230d88d7f Liam R. Howlett 2024-08-30 1176 int vms_gather_munmap_vmas(struct vma_munmap_struct *vms,
> dba14840905f9e Liam R. Howlett 2024-08-30 1177 struct ma_state *mas_detach)
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1178 {
> 01cf21e9e11957 Liam R. Howlett 2024-08-30 1179 struct vm_area_struct *next = NULL;
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1180 int error = -ENOMEM;
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1181
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1182 /*
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1183 * If we need to split any vma, do it now to save pain later.
> 20831cd6f814ea Liam R. Howlett 2024-08-30 1184 * Does it split the first one?
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1185 */
> dba14840905f9e Liam R. Howlett 2024-08-30 1186 if (vms->start > vms->vma->vm_start) {
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1187
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1188 /*
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1189 * Make sure that map_count on return from munmap() will
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1190 * not exceed its limit; but let map_count go just above
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1191 * its limit temporarily, to help free resources as expected.
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1192 */
> dba14840905f9e Liam R. Howlett 2024-08-30 1193 if (vms->end < vms->vma->vm_end &&
> 63fc66f5b6b18f Liam R. Howlett 2024-08-30 1194 vms->vma->vm_mm->map_count >= sysctl_max_map_count)
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1195 goto map_count_exceeded;
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1196
> df2a7df9a9aa32 Pedro Falcato 2024-08-17 1197 /* Don't bother splitting the VMA if we can't unmap it anyway */
> dba14840905f9e Liam R. Howlett 2024-08-30 1198 if (!can_modify_vma(vms->vma)) {
> df2a7df9a9aa32 Pedro Falcato 2024-08-17 1199 error = -EPERM;
> df2a7df9a9aa32 Pedro Falcato 2024-08-17 1200 goto start_split_failed;
> df2a7df9a9aa32 Pedro Falcato 2024-08-17 1201 }
> df2a7df9a9aa32 Pedro Falcato 2024-08-17 1202
> 013545e1b9bca0 Xiao Yang 2024-09-09 1203 error = __split_vma(vms->vmi, vms->vma, vms->start, 1);
> 013545e1b9bca0 Xiao Yang 2024-09-09 1204 if (error)
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1205 goto start_split_failed;
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1206 }
> 17f1ae9b40c6b0 Liam R. Howlett 2024-08-30 1207 vms->prev = vma_prev(vms->vmi);
> 9c3ebeda8fb5a8 Liam R. Howlett 2024-08-30 1208 if (vms->prev)
> 9c3ebeda8fb5a8 Liam R. Howlett 2024-08-30 1209 vms->unmap_start = vms->prev->vm_end;
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1210
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1211 /*
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1212 * Detach a range of VMAs from the mm. Using next as a temp variable as
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1213 * it is always overwritten.
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1214 */
> 17f1ae9b40c6b0 Liam R. Howlett 2024-08-30 1215 for_each_vma_range(*(vms->vmi), next, vms->end) {
> 17f1ae9b40c6b0 Liam R. Howlett 2024-08-30 1216 long nrpages;
> 17f1ae9b40c6b0 Liam R. Howlett 2024-08-30 1217
> df2a7df9a9aa32 Pedro Falcato 2024-08-17 1218 if (!can_modify_vma(next)) {
> df2a7df9a9aa32 Pedro Falcato 2024-08-17 1219 error = -EPERM;
> df2a7df9a9aa32 Pedro Falcato 2024-08-17 1220 goto modify_vma_failed;
> df2a7df9a9aa32 Pedro Falcato 2024-08-17 1221 }
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1222 /* Does it split the end? */
> dba14840905f9e Liam R. Howlett 2024-08-30 1223 if (next->vm_end > vms->end) {
> 013545e1b9bca0 Xiao Yang 2024-09-09 1224 error = __split_vma(vms->vmi, next, vms->end, 0);
> 013545e1b9bca0 Xiao Yang 2024-09-09 1225 if (error)
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1226 goto end_split_failed;
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1227 }
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1228 vma_start_write(next);
> dba14840905f9e Liam R. Howlett 2024-08-30 1229 mas_set(mas_detach, vms->vma_count++);
> 013545e1b9bca0 Xiao Yang 2024-09-09 1230 error = mas_store_gfp(mas_detach, next, GFP_KERNEL);
> 013545e1b9bca0 Xiao Yang 2024-09-09 1231 if (error)
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1232 goto munmap_gather_failed;
> 6898c9039bc8e3 Liam R. Howlett 2024-08-30 1233
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1234 vma_mark_detached(next, true);
> 17f1ae9b40c6b0 Liam R. Howlett 2024-08-30 1235 nrpages = vma_pages(next);
> 17f1ae9b40c6b0 Liam R. Howlett 2024-08-30 1236
> 17f1ae9b40c6b0 Liam R. Howlett 2024-08-30 1237 vms->nr_pages += nrpages;
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1238 if (next->vm_flags & VM_LOCKED)
> 17f1ae9b40c6b0 Liam R. Howlett 2024-08-30 1239 vms->locked_vm += nrpages;
> 17f1ae9b40c6b0 Liam R. Howlett 2024-08-30 1240
> 17f1ae9b40c6b0 Liam R. Howlett 2024-08-30 1241 if (next->vm_flags & VM_ACCOUNT)
> 17f1ae9b40c6b0 Liam R. Howlett 2024-08-30 1242 vms->nr_accounted += nrpages;
> 17f1ae9b40c6b0 Liam R. Howlett 2024-08-30 1243
> 17f1ae9b40c6b0 Liam R. Howlett 2024-08-30 1244 if (is_exec_mapping(next->vm_flags))
> 17f1ae9b40c6b0 Liam R. Howlett 2024-08-30 1245 vms->exec_vm += nrpages;
> 17f1ae9b40c6b0 Liam R. Howlett 2024-08-30 1246 else if (is_stack_mapping(next->vm_flags))
> 17f1ae9b40c6b0 Liam R. Howlett 2024-08-30 1247 vms->stack_vm += nrpages;
> 17f1ae9b40c6b0 Liam R. Howlett 2024-08-30 1248 else if (is_data_mapping(next->vm_flags))
> 17f1ae9b40c6b0 Liam R. Howlett 2024-08-30 1249 vms->data_vm += nrpages;
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1250
> dba14840905f9e Liam R. Howlett 2024-08-30 1251 if (unlikely(vms->uf)) {
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1252 /*
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1253 * If userfaultfd_unmap_prep returns an error the vmas
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1254 * will remain split, but userland will get a
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1255 * highly unexpected error anyway. This is no
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1256 * different than the case where the first of the two
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1257 * __split_vma fails, but we don't undo the first
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1258 * split, despite we could. This is unlikely enough
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1259 * failure that it's not worth optimizing it for.
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 1260 */
> dba14840905f9e Liam R. Howlett 2024-08-30 1261 if (userfaultfd_unmap_prep(next, vms->start, vms->end,
> dba14840905f9e Liam R. Howlett 2024-08-30 1262 vms->uf))
> 49b1b8d6f68310 Lorenzo Stoakes 2024-07-29 @1263 goto userfaultfd_error;
>
> Needs an "error = -ENOMEM;" here. I haven't reviewed this function outside of
> what the zero day bot puts into this email.
Thanks Dan. This is already addressed in v2 [1].
[1]. https://lore.kernel.org/all/20240909125621.1994-1-ice_yangxiao@163.com/
Regards,
Liam
...
next prev parent reply other threads:[~2024-09-10 13:55 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-09 5:02 Xiao Yang
2024-09-09 9:09 ` Lorenzo Stoakes
2024-09-09 12:55 ` 杨晓
2024-09-09 14:00 ` Liam R. Howlett
2024-09-10 13:37 ` Dan Carpenter
2024-09-10 13:55 ` Liam R. Howlett [this message]
2024-09-10 13:55 ` 杨晓
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bfwh67n7cztablduizthnipozy7fagsrsb33vqawv3s3bh53vo@ygxo2rzsomlc \
--to=liam.howlett@oracle.com \
--cc=dan.carpenter@linaro.org \
--cc=ice_yangxiao@163.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=lkp@intel.com \
--cc=ltp@lists.linux.it \
--cc=oe-kbuild-all@lists.linux.dev \
--cc=oe-kbuild@lists.linux.dev \
--cc=oliver.sang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox