On Mon, 2021-12-06 at 09:57 +0000, David Howells wrote:
> Jeff Layton <jlayton@xxxxxxxxxx> 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@xxxxxxxxxx>
