| Message ID | c6cb92379de668be94894f49c2cfa40e73f94d56.1742388096.git.leonro@nvidia.com (mailing list archive) |
|---|---|
| State | Under Review |
| Headers | show |
| Series | [rdma-next] RDMA/core: Silence oversized kvmalloc() warning | expand |
On Wed, Mar 19, 2025 at 02:42:21PM +0200, Leon Romanovsky wrote: > From: Shay Drory <shayd@nvidia.com> > > syzkaller triggered an oversized kvmalloc() warning. > Silence it by adding __GFP_NOWARN. I don't think GFP_NOWARN is the right thing.. We've hit this before and I think we ended up adding a size limit check prior to the kvmalloc to prevent the overflow triggered warning. Jason
On Wed, Mar 19, 2025 at 02:23:49PM -0300, Jason Gunthorpe wrote: > On Wed, Mar 19, 2025 at 02:42:21PM +0200, Leon Romanovsky wrote: > > From: Shay Drory <shayd@nvidia.com> > > > > syzkaller triggered an oversized kvmalloc() warning. > > Silence it by adding __GFP_NOWARN. > > I don't think GFP_NOWARN is the right thing.. > > We've hit this before and I think we ended up adding a size limit > check prior to the kvmalloc to prevent the overflow triggered warning. The size check was needed before this commit was merged: 0708a0afe291 ("mm: Consider __GFP_NOWARN flag for oversized kvmalloc() calls") From that point, the correct solution is simply provide __GFP_NOWARN flag. Thanks > > Jason >
On Wed, Mar 26, 2025 at 12:58:54PM +0200, Leon Romanovsky wrote: > On Wed, Mar 19, 2025 at 02:23:49PM -0300, Jason Gunthorpe wrote: > > On Wed, Mar 19, 2025 at 02:42:21PM +0200, Leon Romanovsky wrote: > > > From: Shay Drory <shayd@nvidia.com> > > > > > > syzkaller triggered an oversized kvmalloc() warning. > > > Silence it by adding __GFP_NOWARN. > > > > I don't think GFP_NOWARN is the right thing.. > > > > We've hit this before and I think we ended up adding a size limit > > check prior to the kvmalloc to prevent the overflow triggered warning. > > The size check was needed before this commit was merged: > 0708a0afe291 ("mm: Consider __GFP_NOWARN flag for oversized kvmalloc() calls") > > From that point, the correct solution is simply provide __GFP_NOWARN flag. I'm not sure, NOWARN removes all warnings, even normal OOM warnings from regually sized allocations which we don't want to remove. Jason
On Mon, Mar 31, 2025, at 20:45, Jason Gunthorpe wrote: > On Wed, Mar 26, 2025 at 12:58:54PM +0200, Leon Romanovsky wrote: >> On Wed, Mar 19, 2025 at 02:23:49PM -0300, Jason Gunthorpe wrote: >> > On Wed, Mar 19, 2025 at 02:42:21PM +0200, Leon Romanovsky wrote: >> > > From: Shay Drory <shayd@nvidia.com> >> > > >> > > syzkaller triggered an oversized kvmalloc() warning. >> > > Silence it by adding __GFP_NOWARN. >> > >> > I don't think GFP_NOWARN is the right thing.. >> > >> > We've hit this before and I think we ended up adding a size limit >> > check prior to the kvmalloc to prevent the overflow triggered warning. >> >> The size check was needed before this commit was merged: >> 0708a0afe291 ("mm: Consider __GFP_NOWARN flag for oversized kvmalloc() calls") >> >> From that point, the correct solution is simply provide __GFP_NOWARN flag. > > I'm not sure, NOWARN removes all warnings, even normal OOM warnings > from regually sized allocations which we don't want to remove. I disagree, this allocation is called from user space. We can safely skip OOM messages and error here will be enough. Thanks > > Jason
On Mon, Mar 31, 2025 at 10:04:05PM +0300, Leon Romanovsky wrote: > > > On Mon, Mar 31, 2025, at 20:45, Jason Gunthorpe wrote: > > On Wed, Mar 26, 2025 at 12:58:54PM +0200, Leon Romanovsky wrote: > >> On Wed, Mar 19, 2025 at 02:23:49PM -0300, Jason Gunthorpe wrote: > >> > On Wed, Mar 19, 2025 at 02:42:21PM +0200, Leon Romanovsky wrote: > >> > > From: Shay Drory <shayd@nvidia.com> > >> > > > >> > > syzkaller triggered an oversized kvmalloc() warning. > >> > > Silence it by adding __GFP_NOWARN. > >> > > >> > I don't think GFP_NOWARN is the right thing.. > >> > > >> > We've hit this before and I think we ended up adding a size limit > >> > check prior to the kvmalloc to prevent the overflow triggered warning. > >> > >> The size check was needed before this commit was merged: > >> 0708a0afe291 ("mm: Consider __GFP_NOWARN flag for oversized kvmalloc() calls") > >> > >> From that point, the correct solution is simply provide __GFP_NOWARN flag. > > > > I'm not sure, NOWARN removes all warnings, even normal OOM warnings > > from regually sized allocations which we don't want to remove. > > I disagree, this allocation is called from user space. We can safely > skip OOM messages and error here will be enough. That is not the standard, we OOM splat on all userspace allocations too. GFP_NOWARN is supposed to be used in cases where the OOM has a recovery and nothing will fail. I think the right thing here is to limit the size, though I'm not really sure what the limit should be. Jason
On Tue, Apr 01, 2025 at 01:35:04PM -0300, Jason Gunthorpe wrote: > On Mon, Mar 31, 2025 at 10:04:05PM +0300, Leon Romanovsky wrote: > > > > > > On Mon, Mar 31, 2025, at 20:45, Jason Gunthorpe wrote: > > > On Wed, Mar 26, 2025 at 12:58:54PM +0200, Leon Romanovsky wrote: > > >> On Wed, Mar 19, 2025 at 02:23:49PM -0300, Jason Gunthorpe wrote: > > >> > On Wed, Mar 19, 2025 at 02:42:21PM +0200, Leon Romanovsky wrote: > > >> > > From: Shay Drory <shayd@nvidia.com> > > >> > > > > >> > > syzkaller triggered an oversized kvmalloc() warning. > > >> > > Silence it by adding __GFP_NOWARN. > > >> > > > >> > I don't think GFP_NOWARN is the right thing.. > > >> > > > >> > We've hit this before and I think we ended up adding a size limit > > >> > check prior to the kvmalloc to prevent the overflow triggered warning. > > >> > > >> The size check was needed before this commit was merged: > > >> 0708a0afe291 ("mm: Consider __GFP_NOWARN flag for oversized kvmalloc() calls") > > >> > > >> From that point, the correct solution is simply provide __GFP_NOWARN flag. > > > > > > I'm not sure, NOWARN removes all warnings, even normal OOM warnings > > > from regually sized allocations which we don't want to remove. > > > > I disagree, this allocation is called from user space. We can safely > > skip OOM messages and error here will be enough. > > That is not the standard, we OOM splat on all userspace allocations > too. > > GFP_NOWARN is supposed to be used in cases where the OOM has a > recovery and nothing will fail. NULL returned back to the user is best way to recover. > > I think the right thing here is to limit the size, though I'm not > really sure what the limit should be. And I still think that NOWARN is the best solution here. Thanks > > Jason
diff --git a/drivers/infiniband/core/umem_odp.c b/drivers/infiniband/core/umem_odp.c index e9fa22d31c23..c48ef6083020 100644 --- a/drivers/infiniband/core/umem_odp.c +++ b/drivers/infiniband/core/umem_odp.c @@ -76,12 +76,14 @@ static inline int ib_init_umem_odp(struct ib_umem_odp *umem_odp, npfns = (end - start) >> PAGE_SHIFT; umem_odp->pfn_list = kvcalloc( - npfns, sizeof(*umem_odp->pfn_list), GFP_KERNEL); + npfns, sizeof(*umem_odp->pfn_list), + GFP_KERNEL | __GFP_NOWARN); if (!umem_odp->pfn_list) return -ENOMEM; umem_odp->dma_list = kvcalloc( - ndmas, sizeof(*umem_odp->dma_list), GFP_KERNEL); + ndmas, sizeof(*umem_odp->dma_list), + GFP_KERNEL | __GFP_NOWARN); if (!umem_odp->dma_list) { ret = -ENOMEM; goto out_pfn_list;