On Wed, 2020-03-04 at 22:26 +0800, Yan, Zheng wrote:
> On 3/4/20 10:05 PM, Jeff Layton wrote:
> > On Wed, 2020-03-04 at 08:55 -0500, Jeff Layton wrote:
> > > On Wed, 2020-03-04 at 21:22 +0800, Yan, Zheng wrote:
> > > > Otherwise ceph_d_delete() may return 1 for the dentry, which makes
> > > > dput() prune the dentry and clear parent dir's complete flag.
> > > >
> > > > Signed-off-by: "Yan, Zheng" <zyan@xxxxxxxxxx>
> > > > ---
> > > > fs/ceph/file.c | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> > > > index 53321bf517c2..671b141aedfe 100644
> > > > --- a/fs/ceph/file.c
> > > > +++ b/fs/ceph/file.c
> > > > @@ -480,6 +480,9 @@ static int try_prep_async_create(struct inode *dir, struct dentry *dentry,
> > > > if (d_in_lookup(dentry)) {
> > > > if (!__ceph_dir_is_complete(ci))
> > > > goto no_async;
> > > > + spin_lock(&dentry->d_lock);
> > > > + di->lease_shared_gen = atomic_read(&ci->i_shared_gen);
> > > > + spin_unlock(&dentry->d_lock);
> > > > } else if (atomic_read(&ci->i_shared_gen) !=
> > > > READ_ONCE(di->lease_shared_gen)) {
> > > > goto no_async;
> > >
> > > Good catch, merged into testing (with small update to changelog
> > > s/mds:/ceph:/)
> > >
> >
> > One related comment though. This patch implies that lease_shared_gen is
> > protected by the d_lock, but it's not held when it's assigned in
> > ceph_lookup. Should it be?
> >
>
> we only assign and compare lease_shared_gen, I think it doesn't matter
> if it's protected by d_lock or not
>
That's the case here too. Do we need the d_lock in
try_prep_async_create? Maybe lease_shared_gen should also be an
atomic_t?
--
Jeff Layton <jlayton@xxxxxxxxxx>
