Messages in this thread | | | Subject | Re: [PATCH 1/2] ceph: conversion to new fscache API | From | Jeff Layton <> | Date | Mon, 06 Dec 2021 05:59:57 -0500 |
| |
On Mon, 2021-12-06 at 09:57 +0000, David Howells wrote: > Jeff Layton <jlayton@kernel.org> wrote: > > > if (!(gfp & __GFP_DIRECT_RECLAIM) || !(gfp & __GFP_FS)) > > There's a function for the first part of this: > > if (!gfpflags_allow_blocking(gfp) || !(gfp & __GFP_FS)) > > > + fsc->fscache = fscache_acquire_volume(name, NULL, 0); > > > > if (fsc->fscache) { > > ent->fscache = fsc->fscache; > > list_add_tail(&ent->list, &ceph_fscache_list); > > It shouldn't really be necessary to have ceph_fscache_list since > fscache_acquire_volume() will do it's own duplicate check. I wonder if I > should make fscache_acquire_volume() return -EEXIST or -EBUSY rather than NULL > in such a case and not print an error, but rather leave that to the filesystem > to display. > > That would allow you to get rid of the ceph_fscache_entry struct also, I > think. >
Returning an error there sounds like a better thing to do.
I'll make the other changes you suggested now. Let me know if you change the fscache_acquire_volume return.
> > +#define FSCACHE_USE_NEW_IO_API > > That doesn't exist anymore. > > > + /* > > + * If we're truncating up, then we should be able to just update > > + * the existing cookie. > > + */ > > + if (size > isize) > > + ceph_fscache_update(inode); > > Might look better to say "expanding" rather than "truncating up". > > David >
-- Jeff Layton <jlayton@kernel.org>
| |