[PATCH v4 04/12] auto-fs: rename d_count field of dentry to d_refcount

[PATCH v4 04/12] auto-fs: rename d_count field of dentry to d_refcount

[Waiman Long]

Because of the d_count name change made in dcache.h, all references
to d_count have to be changed to d_refcount.  There is no change in
logic and everything should just work.

Signed-off-by: Waiman Long <Waiman.Long@hp.com>
---
 fs/autofs4/expire.c |    8 ++++----
 fs/autofs4/root.c   |    2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/autofs4/expire.c b/fs/autofs4/expire.c
index 13ddec9..c0f0a83 100644
--- a/fs/autofs4/expire.c
+++ b/fs/autofs4/expire.c
@@ -109,7 +109,7 @@ cont:
 
 	spin_lock_nested(&q->d_lock, DENTRY_D_LOCK_NESTED);
 	/* Already gone or negative dentry (under construction) - try next */
-	if (q->d_count == 0 || !simple_positive(q)) {
+	if (q->d_refcount == 0 || !simple_positive(q)) {
 		spin_unlock(&q->d_lock);
 		next = q->d_u.d_child.next;
 		goto cont;
@@ -267,7 +267,7 @@ static int autofs4_tree_busy(struct vfsmount *mnt,
 			else
 				ino_count++;
 
-			if (p->d_count > ino_count) {
+			if (p->d_refcount > ino_count) {
 				top_ino->last_used = jiffies;
 				dput(p);
 				return 1;
@@ -409,7 +409,7 @@ struct dentry *autofs4_expire_indirect(struct super_block *sb,
 		if (!exp_leaves) {
 			/* Path walk currently on this dentry? */
 			ino_count = atomic_read(&ino->count) + 1;
-			if (dentry->d_count > ino_count)
+			if (dentry->d_refcount > ino_count)
 				goto next;
 
 			if (!autofs4_tree_busy(mnt, dentry, timeout, do_now)) {
@@ -423,7 +423,7 @@ struct dentry *autofs4_expire_indirect(struct super_block *sb,
 		} else {
 			/* Path walk currently on this dentry? */
 			ino_count = atomic_read(&ino->count) + 1;
-			if (dentry->d_count > ino_count)
+			if (dentry->d_refcount > ino_count)
 				goto next;
 
 			expired = autofs4_check_leaves(mnt, dentry, timeout, do_now);
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index 085da86..85b169e 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -179,7 +179,7 @@ static struct dentry *autofs4_lookup_active(struct dentry *dentry)
 		spin_lock(&active->d_lock);
 
 		/* Already gone? */
-		if (active->d_count == 0)
+		if (active->d_refcount == 0)
 			goto next;
 
 		qstr = &active->d_name;
-- 
1.7.1

Re: [PATCH v4 04/12] auto-fs: rename d_count field of dentry to d_refcount

[Linus Torvalds]

On Wed, Jul 3, 2013 at 8:33 PM, Waiman Long <Waiman.Long@hp.com> wrote:
> Because of the d_count name change made in dcache.h, all references
> to d_count have to be changed to d_refcount.  There is no change in
> logic and everything should just work.

These filesystem patches need to be just joined into the same patch
that does the d_count -> d_refcount change.

Otherwise the kernel won't build in lots of configurations for some
commits, which makes things like bisecting much more painful than it
should be.

So we can't do piece-meal changes that break the build for parts of the tree.

              Linus

Re: [PATCH v4 04/12] auto-fs: rename d_count field of dentry to d_refcount

[Al Viro]

On Wed, Jul 03, 2013 at 08:50:07PM -0700, Linus Torvalds wrote:
> On Wed, Jul 3, 2013 at 8:33 PM, Waiman Long <Waiman.Long@hp.com> wrote:
> > Because of the d_count name change made in dcache.h, all references
> > to d_count have to be changed to d_refcount.  There is no change in
> > logic and everything should just work.
> 
> These filesystem patches need to be just joined into the same patch
> that does the d_count -> d_refcount change.
> 
> Otherwise the kernel won't build in lots of configurations for some
> commits, which makes things like bisecting much more painful than it
> should be.
> 
> So we can't do piece-meal changes that break the build for parts of the tree.

Frankly, my preference here would be to add static inline unsigned d_count(...)
and convert the uses of ->d_count outside of fs/{dcache.c,namei.c} and
include/linux/dcache.c to it as the first commit.  All users outside those
are readers, so there's no point playing with macros in this case...

Re: [PATCH v4 04/12] auto-fs: rename d_count field of dentry to d_refcount

[Waiman Long]

On 07/03/2013 11:50 PM, Linus Torvalds wrote:
> On Wed, Jul 3, 2013 at 8:33 PM, Waiman Long<Waiman.Long@hp.com>  wrote:
>> Because of the d_count name change made in dcache.h, all references
>> to d_count have to be changed to d_refcount.  There is no change in
>> logic and everything should just work.
> These filesystem patches need to be just joined into the same patch
> that does the d_count ->  d_refcount change.
>
> Otherwise the kernel won't build in lots of configurations for some
> commits, which makes things like bisecting much more painful than it
> should be.
>
> So we can't do piece-meal changes that break the build for parts of the tree.
>
>                Linus

I could change patch 3 so that I keep the d_count name, but #define 
d_refcount to d_count. In that way, I can do piece-meal changes without 
breaking the build. Alternatively, I could collapse patches 3-11 into a 
single big patch which will be harder to review.

Regards,
Longman

Re: [PATCH v4 04/12] auto-fs: rename d_count field of dentry to d_refcount

[Waiman Long]

On 07/04/2013 12:59 AM, Al Viro wrote:
> On Wed, Jul 03, 2013 at 08:50:07PM -0700, Linus Torvalds wrote:
>> On Wed, Jul 3, 2013 at 8:33 PM, Waiman Long<Waiman.Long@hp.com>  wrote:
>>> Because of the d_count name change made in dcache.h, all references
>>> to d_count have to be changed to d_refcount.  There is no change in
>>> logic and everything should just work.
>> These filesystem patches need to be just joined into the same patch
>> that does the d_count ->  d_refcount change.
>>
>> Otherwise the kernel won't build in lots of configurations for some
>> commits, which makes things like bisecting much more painful than it
>> should be.
>>
>> So we can't do piece-meal changes that break the build for parts of the tree.
> Frankly, my preference here would be to add static inline unsigned d_count(...)
> and convert the uses of ->d_count outside of fs/{dcache.c,namei.c} and
> include/linux/dcache.c to it as the first commit.  All users outside those
> are readers, so there's no point playing with macros in this case...

Yes, I could do that.

Regards,
Longman

Re: [PATCH v4 04/12] auto-fs: rename d_count field of dentry to d_refcount

[Linus Torvalds]

On Thu, Jul 4, 2013 at 7:47 AM, Waiman Long <waiman.long@hp.com> wrote:
>
> I could change patch 3 so that I keep the d_count name, but #define
> d_refcount to d_count. In that way, I can do piece-meal changes without
> breaking the build. Alternatively, I could collapse patches 3-11 into a
> single big patch which will be harder to review.

Since there are many fewer d_count users than there are d_lock users,
I think collapsing things is the right thing to do.

That said, I think Al is right that for all those filesystem uses, we
might actually be much better off with a helper function looking at
d_count, with no macros etc, since they are purely about reading the
count.

So maybe the right thing to do is to add a

   static inline int d_count(struct dentry *dentry) { return dentry->d_count; }

helper function *first*, and just say "filesystems should never access
d_count directly", and make the few filesystem users use this helper
function first. That way we can do that as independent commits to
prepare for the switch-over.

Then when the switch-over happens, we just change "d_count" in that
helper function, and it has no filesystem impact at all.

But fs/dcache.c and fs/namei.c that actually really know about and
modify d_count would not use that helper function. It would purely be
about isolating filesystems from these kinds of internal
implementation issues: fs/cache.c and fs/namei.c are all *about* those
internal issues, so they shouldn't be isolated..

           Linus

Re: [PATCH v4 04/12] auto-fs: rename d_count field of dentry to d_refcount

[Thomas Gleixner]

On Thu, 4 Jul 2013, Linus Torvalds wrote:

> On Thu, Jul 4, 2013 at 7:47 AM, Waiman Long <waiman.long@hp.com> wrote:
> >
> > I could change patch 3 so that I keep the d_count name, but #define
> > d_refcount to d_count. In that way, I can do piece-meal changes without
> > breaking the build. Alternatively, I could collapse patches 3-11 into a
> > single big patch which will be harder to review.
> 
> Since there are many fewer d_count users than there are d_lock users,
> I think collapsing things is the right thing to do.
> 
> That said, I think Al is right that for all those filesystem uses, we
> might actually be much better off with a helper function looking at
> d_count, with no macros etc, since they are purely about reading the
> count.
> 
> So maybe the right thing to do is to add a
> 
>    static inline int d_count(struct dentry *dentry) { return dentry->d_count; }
> 
> helper function *first*, and just say "filesystems should never access
> d_count directly", and make the few filesystem users use this helper
> function first. That way we can do that as independent commits to
> prepare for the switch-over.
> 
> Then when the switch-over happens, we just change "d_count" in that
> helper function, and it has no filesystem impact at all.
> 
> But fs/dcache.c and fs/namei.c that actually really know about and
> modify d_count would not use that helper function. It would purely be
> about isolating filesystems from these kinds of internal
> implementation issues: fs/cache.c and fs/namei.c are all *about* those
> internal issues, so they shouldn't be isolated..

And all of this can sanely be done with coccinelle. Julia will
certainly help to get the scripts right.

Thanks,

	tglx