| Message ID | 20220224160119.1034749-1-amir73il@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nfsd: more robust allocation failure handling in nfsd_file_cache_init | expand |
On Thu, 2022-02-24 at 18:01 +0200, Amir Goldstein wrote: > The nfsd file cache table can be pretty large and its allocation > may require as many as 80 contigious pages. > > Employ the same fix that was employed for similar issue that was > reported for the reply cache hash table allocation several years ago > by commit 8f97514b423a ("nfsd: more robust allocation failure handling > in nfsd_reply_cache_init"). > > Fixes: 65294c1f2c5e ("nfsd: add a new struct file caching facility to nfsd") > Link: https://lore.kernel.org/linux-nfs/e3cdaeec85a6cfec980e87fc294327c0381c1778.camel@kernel.org/ > Suggested-by: Jeff Layton <jlayton@kernel.org> > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- > fs/nfsd/filecache.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c > index 8bc807c5fea4..b75cd6d1e331 100644 > --- a/fs/nfsd/filecache.c > +++ b/fs/nfsd/filecache.c > @@ -60,6 +60,9 @@ static struct fsnotify_group *nfsd_file_fsnotify_group; > static atomic_long_t nfsd_filecache_count; > static struct delayed_work nfsd_filecache_laundrette; > > +#define NFSD_FILE_HASHTBL_SIZE \ > + array_size(NFSD_FILE_HASH_SIZE, sizeof(*nfsd_file_hashtbl)) > + > static void nfsd_file_gc(void); > > static void > @@ -632,8 +635,7 @@ nfsd_file_cache_init(void) > if (!nfsd_filecache_wq) > goto out; > > - nfsd_file_hashtbl = kcalloc(NFSD_FILE_HASH_SIZE, > - sizeof(*nfsd_file_hashtbl), GFP_KERNEL); > + nfsd_file_hashtbl = kvzalloc(NFSD_FILE_HASHTBL_SIZE, GFP_KERNEL); > if (!nfsd_file_hashtbl) { > pr_err("nfsd: unable to allocate nfsd_file_hashtbl\n"); > goto out_err; You'd also need to change to kvfree(), in case it does end up being vmalloc'ed. Also, I don't see that the new constant adds anything -- you could just use kvcalloc().
Hi Jeff, Amir - > On Feb 24, 2022, at 11:01 AM, Amir Goldstein <amir73il@gmail.com> wrote: > > The nfsd file cache table can be pretty large and its allocation > may require as many as 80 contigious pages. > > Employ the same fix that was employed for similar issue that was > reported for the reply cache hash table allocation several years ago > by commit 8f97514b423a ("nfsd: more robust allocation failure handling > in nfsd_reply_cache_init"). > > Fixes: 65294c1f2c5e ("nfsd: add a new struct file caching facility to nfsd") > Link: https://lore.kernel.org/linux-nfs/e3cdaeec85a6cfec980e87fc294327c0381c1778.camel@kernel.org/ > Suggested-by: Jeff Layton <jlayton@kernel.org> > Signed-off-by: Amir Goldstein <amir73il@gmail.com> IMO, this approach is a band-aid, as was 8f97514b423a. I note that not as a rejection, but rather to point out that more work is needed in this area. Big hash tables are inflexible. I'll go with this one because you provided a patch description ;-) But, as you noted, you need to fix up the kfree() paths too. > --- > fs/nfsd/filecache.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c > index 8bc807c5fea4..b75cd6d1e331 100644 > --- a/fs/nfsd/filecache.c > +++ b/fs/nfsd/filecache.c > @@ -60,6 +60,9 @@ static struct fsnotify_group *nfsd_file_fsnotify_group; > static atomic_long_t nfsd_filecache_count; > static struct delayed_work nfsd_filecache_laundrette; > > +#define NFSD_FILE_HASHTBL_SIZE \ > + array_size(NFSD_FILE_HASH_SIZE, sizeof(*nfsd_file_hashtbl)) > + > static void nfsd_file_gc(void); > > static void > @@ -632,8 +635,7 @@ nfsd_file_cache_init(void) > if (!nfsd_filecache_wq) > goto out; > > - nfsd_file_hashtbl = kcalloc(NFSD_FILE_HASH_SIZE, > - sizeof(*nfsd_file_hashtbl), GFP_KERNEL); > + nfsd_file_hashtbl = kvzalloc(NFSD_FILE_HASHTBL_SIZE, GFP_KERNEL); > if (!nfsd_file_hashtbl) { > pr_err("nfsd: unable to allocate nfsd_file_hashtbl\n"); > goto out_err; > -- > 2.25.1 > -- Chuck Lever
diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c index 8bc807c5fea4..b75cd6d1e331 100644 --- a/fs/nfsd/filecache.c +++ b/fs/nfsd/filecache.c @@ -60,6 +60,9 @@ static struct fsnotify_group *nfsd_file_fsnotify_group; static atomic_long_t nfsd_filecache_count; static struct delayed_work nfsd_filecache_laundrette; +#define NFSD_FILE_HASHTBL_SIZE \ + array_size(NFSD_FILE_HASH_SIZE, sizeof(*nfsd_file_hashtbl)) + static void nfsd_file_gc(void); static void @@ -632,8 +635,7 @@ nfsd_file_cache_init(void) if (!nfsd_filecache_wq) goto out; - nfsd_file_hashtbl = kcalloc(NFSD_FILE_HASH_SIZE, - sizeof(*nfsd_file_hashtbl), GFP_KERNEL); + nfsd_file_hashtbl = kvzalloc(NFSD_FILE_HASHTBL_SIZE, GFP_KERNEL); if (!nfsd_file_hashtbl) { pr_err("nfsd: unable to allocate nfsd_file_hashtbl\n"); goto out_err;
The nfsd file cache table can be pretty large and its allocation may require as many as 80 contigious pages. Employ the same fix that was employed for similar issue that was reported for the reply cache hash table allocation several years ago by commit 8f97514b423a ("nfsd: more robust allocation failure handling in nfsd_reply_cache_init"). Fixes: 65294c1f2c5e ("nfsd: add a new struct file caching facility to nfsd") Link: https://lore.kernel.org/linux-nfs/e3cdaeec85a6cfec980e87fc294327c0381c1778.camel@kernel.org/ Suggested-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- fs/nfsd/filecache.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)