* [PATCH] mds: update dentry lease for async create
@ 2020-03-04 13:22 Yan, Zheng
2020-03-04 13:55 ` Jeff Layton
0 siblings, 1 reply; 5+ messages in thread
From: Yan, Zheng @ 2020-03-04 13:22 UTC (permalink / raw)
To: ceph-devel; +Cc: jlayton, Yan, Zheng
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@redhat.com>
---
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;
--
2.21.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] mds: update dentry lease for async create
2020-03-04 13:22 [PATCH] mds: update dentry lease for async create Yan, Zheng
@ 2020-03-04 13:55 ` Jeff Layton
2020-03-04 14:05 ` Jeff Layton
0 siblings, 1 reply; 5+ messages in thread
From: Jeff Layton @ 2020-03-04 13:55 UTC (permalink / raw)
To: Yan, Zheng, ceph-devel
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@redhat.com>
> ---
> 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:/)
Thanks!
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mds: update dentry lease for async create
2020-03-04 13:55 ` Jeff Layton
@ 2020-03-04 14:05 ` Jeff Layton
2020-03-04 14:26 ` Yan, Zheng
0 siblings, 1 reply; 5+ messages in thread
From: Jeff Layton @ 2020-03-04 14:05 UTC (permalink / raw)
To: Yan, Zheng, ceph-devel
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@redhat.com>
> > ---
> > 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?
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mds: update dentry lease for async create
2020-03-04 14:05 ` Jeff Layton
@ 2020-03-04 14:26 ` Yan, Zheng
2020-03-04 14:50 ` Jeff Layton
0 siblings, 1 reply; 5+ messages in thread
From: Yan, Zheng @ 2020-03-04 14:26 UTC (permalink / raw)
To: Jeff Layton, ceph-devel
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@redhat.com>
>>> ---
>>> 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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mds: update dentry lease for async create
2020-03-04 14:26 ` Yan, Zheng
@ 2020-03-04 14:50 ` Jeff Layton
0 siblings, 0 replies; 5+ messages in thread
From: Jeff Layton @ 2020-03-04 14:50 UTC (permalink / raw)
To: Yan, Zheng, ceph-devel
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@redhat.com>
> > > > ---
> > > > 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@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-03-04 14:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-04 13:22 [PATCH] mds: update dentry lease for async create Yan, Zheng
2020-03-04 13:55 ` Jeff Layton
2020-03-04 14:05 ` Jeff Layton
2020-03-04 14:26 ` Yan, Zheng
2020-03-04 14:50 ` Jeff Layton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox