Re: [GIT PULL] Ceph fixes for 5.1-rc7

From: Al Viro
Date: Fri Apr 26 2019 - 17:28:51 EST


On Fri, Apr 26, 2019 at 04:49:24PM -0400, Jeff Layton wrote:

> Got it, thanks. Why use an atomic_t for the refcount if it's always
> accessed under spinlock?

_Which_ spinlock? The whole reason for refcounts is that many dentries
might end up with shared external name. So ->d_lock on one of them
won't do anything to accesses via another...

> > What I don't understand is why would anyone want to mess with
> > name snapshots for dentry_path() lookalikes...
>
> Mostly because the place where the ceph code needs to use and free these
> strings is rather far removed from where they are created. It simplifies
> that part if we can access and free them all in the same way.
>
> I was planning to just use name_snapshots universally for that purpose,
> but they have a rather specific method of freeing things that is hard to
> duplicate if you don't have a dentry to clone.
>
> Probably that means I'm approaching this problem in the wrong way and
> need to do it differently.

For short names snapshot will put the copy into your variable, normally
on the stack frame. So it doesn't really help if you want to keep that
stuff around. Again, dentries with short names have those stored inside
the dentry and for anything long-term you will need to copy that data;
otherwise rename() will happily change those under you.

If you want to deal with pathnames, do __getname() + a loop similar to
that in dentry_path() et.al. All there is to it. Snapshots are
for situations when you want a reasonably short-term access to
name of specific dentry you have a reference to and do not want
to do any allocations.