diff mbox series

[17/22] get rid of passing callbacks to ceph __dentry_leases_walk()

Message ID 20231220052925.GP1674809@ZenIV (mailing list archive)
State New, archived
Headers show
Series PATCH 01/22] hostfs: use d_splice_alias() calling conventions to simplify failure exits | expand

Commit Message

Al Viro Dec. 20, 2023, 5:29 a.m. UTC 
__dentry_leases_walk() is gets a callback and calls it for
a bunch of denties; there are exactly two callers and
we already have a flag telling them apart - lwc->dir_lease.

Seeing that indirect calls are costly these days, let's
get rid of the callback and just call the right function
directly.  Has a side benefit of saner signatures...

Signed-off-by Al Viro <viro@zeniv.linux.org.uk>
---
 fs/ceph/dir.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

Comments

Xiubo Li Dec. 21, 2023, 12:45 a.m. UTC | #1 
On 12/20/23 13:29, Al Viro wrote:
> __dentry_leases_walk() is gets a callback and calls it for
> a bunch of denties; there are exactly two callers and
> we already have a flag telling them apart - lwc->dir_lease.
>
> Seeing that indirect calls are costly these days, let's
> get rid of the callback and just call the right function
> directly.  Has a side benefit of saner signatures...
>
> Signed-off-by Al Viro <viro@zeniv.linux.org.uk>
> ---
>   fs/ceph/dir.c | 21 +++++++++++++--------
>   1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index 91709934c8b1..768158743750 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -1593,10 +1593,12 @@ struct ceph_lease_walk_control {
>   	unsigned long dir_lease_ttl;
>   };
>   
> +static int __dir_lease_check(const struct dentry *, struct ceph_lease_walk_control *);
> +static int __dentry_lease_check(const struct dentry *);
> +
>   static unsigned long
>   __dentry_leases_walk(struct ceph_mds_client *mdsc,
> -		     struct ceph_lease_walk_control *lwc,
> -		     int (*check)(struct dentry*, void*))
> +		     struct ceph_lease_walk_control *lwc)
>   {
>   	struct ceph_dentry_info *di, *tmp;
>   	struct dentry *dentry, *last = NULL;
> @@ -1624,7 +1626,10 @@ __dentry_leases_walk(struct ceph_mds_client *mdsc,
>   			goto next;
>   		}
>   
> -		ret = check(dentry, lwc);
> +		if (lwc->dir_lease)
> +			ret = __dir_lease_check(dentry, lwc);
> +		else
> +			ret = __dentry_lease_check(dentry);
>   		if (ret & TOUCH) {
>   			/* move it into tail of dir lease list */
>   			__dentry_dir_lease_touch(mdsc, di);
> @@ -1681,7 +1686,7 @@ __dentry_leases_walk(struct ceph_mds_client *mdsc,
>   	return freed;
>   }
>   
> -static int __dentry_lease_check(struct dentry *dentry, void *arg)
> +static int __dentry_lease_check(const struct dentry *dentry)
>   {
>   	struct ceph_dentry_info *di = ceph_dentry(dentry);
>   	int ret;
> @@ -1696,9 +1701,9 @@ static int __dentry_lease_check(struct dentry *dentry, void *arg)
>   	return DELETE;
>   }
>   
> -static int __dir_lease_check(struct dentry *dentry, void *arg)
> +static int __dir_lease_check(const struct dentry *dentry,
> +			     struct ceph_lease_walk_control *lwc)
>   {
> -	struct ceph_lease_walk_control *lwc = arg;
>   	struct ceph_dentry_info *di = ceph_dentry(dentry);
>   
>   	int ret = __dir_lease_try_check(dentry);
> @@ -1737,7 +1742,7 @@ int ceph_trim_dentries(struct ceph_mds_client *mdsc)
>   
>   	lwc.dir_lease = false;
>   	lwc.nr_to_scan  = CEPH_CAPS_PER_RELEASE * 2;
> -	freed = __dentry_leases_walk(mdsc, &lwc, __dentry_lease_check);
> +	freed = __dentry_leases_walk(mdsc, &lwc);
>   	if (!lwc.nr_to_scan) /* more invalid leases */
>   		return -EAGAIN;
>   
> @@ -1747,7 +1752,7 @@ int ceph_trim_dentries(struct ceph_mds_client *mdsc)
>   	lwc.dir_lease = true;
>   	lwc.expire_dir_lease = freed < count;
>   	lwc.dir_lease_ttl = mdsc->fsc->mount_options->caps_wanted_delay_max * HZ;
> -	freed +=__dentry_leases_walk(mdsc, &lwc, __dir_lease_check);
> +	freed +=__dentry_leases_walk(mdsc, &lwc);
>   	if (!lwc.nr_to_scan) /* more to check */
>   		return -EAGAIN;
>   

Reviewed-by: Xiubo Li <xiubli@redhat.com>

Al,

I think these two ceph patches won't be dependent by your following 
patches,  right ?

If so we can apply them to ceph-client tree and run more tests.


Thanks!
Al Viro Dec. 21, 2023, 1:12 a.m. UTC | #2 
On Thu, Dec 21, 2023 at 08:45:18AM +0800, Xiubo Li wrote:
> Al,
> 
> I think these two ceph patches won't be dependent by your following
> patches,  right ?
> 
> If so we can apply them to ceph-client tree and run more tests.

All of them are mutually independent, and if you are willing to take
them through your tree - all the better.
Xiubo Li Dec. 21, 2023, 1:16 a.m. UTC | #3 
On 12/21/23 09:12, Al Viro wrote:
> On Thu, Dec 21, 2023 at 08:45:18AM +0800, Xiubo Li wrote:
>> Al,
>>
>> I think these two ceph patches won't be dependent by your following
>> patches,  right ?
>>
>> If so we can apply them to ceph-client tree and run more tests.
> All of them are mutually independent, and if you are willing to take
> them through your tree - all the better.
>
Sure, I will apply them into the ceph-client repo. And will run the 
tests for them.

Thanks Al.

- Xiubo
diff mbox series

Patch

diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 91709934c8b1..768158743750 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1593,10 +1593,12 @@  struct ceph_lease_walk_control {
 	unsigned long dir_lease_ttl;
 };
 
+static int __dir_lease_check(const struct dentry *, struct ceph_lease_walk_control *);
+static int __dentry_lease_check(const struct dentry *);
+
 static unsigned long
 __dentry_leases_walk(struct ceph_mds_client *mdsc,
-		     struct ceph_lease_walk_control *lwc,
-		     int (*check)(struct dentry*, void*))
+		     struct ceph_lease_walk_control *lwc)
 {
 	struct ceph_dentry_info *di, *tmp;
 	struct dentry *dentry, *last = NULL;
@@ -1624,7 +1626,10 @@  __dentry_leases_walk(struct ceph_mds_client *mdsc,
 			goto next;
 		}
 
-		ret = check(dentry, lwc);
+		if (lwc->dir_lease)
+			ret = __dir_lease_check(dentry, lwc);
+		else
+			ret = __dentry_lease_check(dentry);
 		if (ret & TOUCH) {
 			/* move it into tail of dir lease list */
 			__dentry_dir_lease_touch(mdsc, di);
@@ -1681,7 +1686,7 @@  __dentry_leases_walk(struct ceph_mds_client *mdsc,
 	return freed;
 }
 
-static int __dentry_lease_check(struct dentry *dentry, void *arg)
+static int __dentry_lease_check(const struct dentry *dentry)
 {
 	struct ceph_dentry_info *di = ceph_dentry(dentry);
 	int ret;
@@ -1696,9 +1701,9 @@  static int __dentry_lease_check(struct dentry *dentry, void *arg)
 	return DELETE;
 }
 
-static int __dir_lease_check(struct dentry *dentry, void *arg)
+static int __dir_lease_check(const struct dentry *dentry,
+			     struct ceph_lease_walk_control *lwc)
 {
-	struct ceph_lease_walk_control *lwc = arg;
 	struct ceph_dentry_info *di = ceph_dentry(dentry);
 
 	int ret = __dir_lease_try_check(dentry);
@@ -1737,7 +1742,7 @@  int ceph_trim_dentries(struct ceph_mds_client *mdsc)
 
 	lwc.dir_lease = false;
 	lwc.nr_to_scan  = CEPH_CAPS_PER_RELEASE * 2;
-	freed = __dentry_leases_walk(mdsc, &lwc, __dentry_lease_check);
+	freed = __dentry_leases_walk(mdsc, &lwc);
 	if (!lwc.nr_to_scan) /* more invalid leases */
 		return -EAGAIN;
 
@@ -1747,7 +1752,7 @@  int ceph_trim_dentries(struct ceph_mds_client *mdsc)
 	lwc.dir_lease = true;
 	lwc.expire_dir_lease = freed < count;
 	lwc.dir_lease_ttl = mdsc->fsc->mount_options->caps_wanted_delay_max * HZ;
-	freed +=__dentry_leases_walk(mdsc, &lwc, __dir_lease_check);
+	freed +=__dentry_leases_walk(mdsc, &lwc);
 	if (!lwc.nr_to_scan) /* more to check */
 		return -EAGAIN;