All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs_repair: stop using ->data_entry_p()
@ 2020-01-17 23:17 Eric Sandeen
  2020-01-18  4:39 ` Darrick J. Wong
  2020-01-20  8:58 ` Christoph Hellwig
  0 siblings, 2 replies; 6+ messages in thread
From: Eric Sandeen @ 2020-01-17 23:17 UTC (permalink / raw)
  To: linux-xfs, Darrick J. Wong

The ->data_entry_p() op went away in v5.5 kernelspace, so rework
xfs_repair to use ->data_entry_offset instead, in preparation
for the v5.5 libxfs backport.

This could later be cleaned up to use offsets as was done
in kernel commit 8073af5153c for example.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

I'll munge this patch in mid-libxfs-sync, just before the
->data_entry_p removal patch.

diff --git a/repair/dir2.c b/repair/dir2.c
index 4ac0084e..2494f3c4 100644
--- a/repair/dir2.c
+++ b/repair/dir2.c
@@ -580,7 +580,7 @@ process_dir2_data(
 
 	d = bp->b_addr;
 	bf = M_DIROPS(mp)->data_bestfree_p(d);
-	ptr = (char *)M_DIROPS(mp)->data_entry_p(d);
+	ptr = (char *)d + M_DIROPS(mp)->data_entry_offset;
 	badbest = lastfree = freeseen = 0;
 	if (be16_to_cpu(bf[0].length) == 0) {
 		badbest |= be16_to_cpu(bf[0].offset) != 0;
@@ -646,7 +646,7 @@ process_dir2_data(
 			do_warn(_("\twould junk block\n"));
 		return 1;
 	}
-	ptr = (char *)M_DIROPS(mp)->data_entry_p(d);
+	ptr = (char *)d + M_DIROPS(mp)->data_entry_offset;
 	/*
 	 * Process the entries now.
 	 */
diff --git a/repair/phase6.c b/repair/phase6.c
index 91d208a6..d61b2ae7 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -1530,7 +1530,7 @@ longform_dir2_entry_check_data(
 
 	bp = *bpp;
 	d = bp->b_addr;
-	ptr = (char *)M_DIROPS(mp)->data_entry_p(d);
+	ptr = (char *)d + M_DIROPS(mp)->data_entry_offset;
 	nbad = 0;
 	needscan = needlog = 0;
 	junkit = 0;
@@ -1590,7 +1590,7 @@ longform_dir2_entry_check_data(
 				break;
 
 			/* check for block with no data entries */
-			if ((ptr == (char *)M_DIROPS(mp)->data_entry_p(d)) &&
+			if ((ptr == (char *)d + M_DIROPS(mp)->data_entry_offset) &&
 			    (ptr + be16_to_cpu(dup->length) >= endptr)) {
 				junkit = 1;
 				*num_illegal += 1;
@@ -1659,7 +1659,7 @@ longform_dir2_entry_check_data(
 			do_warn(_("would fix magic # to %#x\n"), wantmagic);
 	}
 	lastfree = 0;
-	ptr = (char *)M_DIROPS(mp)->data_entry_p(d);
+	ptr = (char *)d + M_DIROPS(mp)->data_entry_offset;
 	/*
 	 * look at each entry.  reference inode pointed to by each
 	 * entry in the incore inode tree.
@@ -1834,7 +1834,7 @@ longform_dir2_entry_check_data(
 			       (dep->name[0] == '.' && dep->namelen == 1));
 			add_inode_ref(current_irec, current_ino_offset);
 			if (da_bno != 0 ||
-			    dep != M_DIROPS(mp)->data_entry_p(d)) {
+			    dep != (void *)d + M_DIROPS(mp)->data_entry_offset) {
 				/* "." should be the first entry */
 				nbad++;
 				if (entry_junked(


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] xfs_repair: stop using ->data_entry_p()
  2020-01-17 23:17 [PATCH] xfs_repair: stop using ->data_entry_p() Eric Sandeen
@ 2020-01-18  4:39 ` Darrick J. Wong
  2020-01-20 14:29   ` Eric Sandeen
  2020-01-20  8:58 ` Christoph Hellwig
  1 sibling, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2020-01-18  4:39 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: linux-xfs

On Fri, Jan 17, 2020 at 05:17:11PM -0600, Eric Sandeen wrote:
> The ->data_entry_p() op went away in v5.5 kernelspace, so rework
> xfs_repair to use ->data_entry_offset instead, in preparation
> for the v5.5 libxfs backport.
> 
> This could later be cleaned up to use offsets as was done
> in kernel commit 8073af5153c for example.

See, now that you've said that, I start wondering why not do that?

:D

> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> I'll munge this patch in mid-libxfs-sync, just before the
> ->data_entry_p removal patch.
> 
> diff --git a/repair/dir2.c b/repair/dir2.c
> index 4ac0084e..2494f3c4 100644
> --- a/repair/dir2.c
> +++ b/repair/dir2.c
> @@ -580,7 +580,7 @@ process_dir2_data(
>  
>  	d = bp->b_addr;
>  	bf = M_DIROPS(mp)->data_bestfree_p(d);
> -	ptr = (char *)M_DIROPS(mp)->data_entry_p(d);
> +	ptr = (char *)d + M_DIROPS(mp)->data_entry_offset;
>  	badbest = lastfree = freeseen = 0;
>  	if (be16_to_cpu(bf[0].length) == 0) {
>  		badbest |= be16_to_cpu(bf[0].offset) != 0;
> @@ -646,7 +646,7 @@ process_dir2_data(
>  			do_warn(_("\twould junk block\n"));
>  		return 1;
>  	}
> -	ptr = (char *)M_DIROPS(mp)->data_entry_p(d);
> +	ptr = (char *)d + M_DIROPS(mp)->data_entry_offset;
>  	/*
>  	 * Process the entries now.
>  	 */
> diff --git a/repair/phase6.c b/repair/phase6.c
> index 91d208a6..d61b2ae7 100644
> --- a/repair/phase6.c
> +++ b/repair/phase6.c
> @@ -1530,7 +1530,7 @@ longform_dir2_entry_check_data(
>  
>  	bp = *bpp;
>  	d = bp->b_addr;
> -	ptr = (char *)M_DIROPS(mp)->data_entry_p(d);
> +	ptr = (char *)d + M_DIROPS(mp)->data_entry_offset;
>  	nbad = 0;
>  	needscan = needlog = 0;
>  	junkit = 0;
> @@ -1590,7 +1590,7 @@ longform_dir2_entry_check_data(
>  				break;
>  
>  			/* check for block with no data entries */
> -			if ((ptr == (char *)M_DIROPS(mp)->data_entry_p(d)) &&
> +			if ((ptr == (char *)d + M_DIROPS(mp)->data_entry_offset) &&
>  			    (ptr + be16_to_cpu(dup->length) >= endptr)) {
>  				junkit = 1;
>  				*num_illegal += 1;
> @@ -1659,7 +1659,7 @@ longform_dir2_entry_check_data(
>  			do_warn(_("would fix magic # to %#x\n"), wantmagic);
>  	}
>  	lastfree = 0;
> -	ptr = (char *)M_DIROPS(mp)->data_entry_p(d);
> +	ptr = (char *)d + M_DIROPS(mp)->data_entry_offset;
>  	/*
>  	 * look at each entry.  reference inode pointed to by each
>  	 * entry in the incore inode tree.
> @@ -1834,7 +1834,7 @@ longform_dir2_entry_check_data(
>  			       (dep->name[0] == '.' && dep->namelen == 1));
>  			add_inode_ref(current_irec, current_ino_offset);
>  			if (da_bno != 0 ||
> -			    dep != M_DIROPS(mp)->data_entry_p(d)) {
> +			    dep != (void *)d + M_DIROPS(mp)->data_entry_offset) {

Er.... void pointer arithmetic?

(Though I really do wish the original author of the kernel patchset had
supplied a xfsprogs port of the kernel patches so we maintainers don't
get stuck doing all the porting work...)

--D

>  				/* "." should be the first entry */
>  				nbad++;
>  				if (entry_junked(
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xfs_repair: stop using ->data_entry_p()
  2020-01-17 23:17 [PATCH] xfs_repair: stop using ->data_entry_p() Eric Sandeen
  2020-01-18  4:39 ` Darrick J. Wong
@ 2020-01-20  8:58 ` Christoph Hellwig
  1 sibling, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2020-01-20  8:58 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: linux-xfs, Darrick J. Wong

On Fri, Jan 17, 2020 at 05:17:11PM -0600, Eric Sandeen wrote:
> The ->data_entry_p() op went away in v5.5 kernelspace, so rework
> xfs_repair to use ->data_entry_offset instead, in preparation
> for the v5.5 libxfs backport.
> 
> This could later be cleaned up to use offsets as was done
> in kernel commit 8073af5153c for example.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> I'll munge this patch in mid-libxfs-sync, just before the
> ->data_entry_p removal patch.

Looks good, and I can give the cleanup a try once I find a little time.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xfs_repair: stop using ->data_entry_p()
  2020-01-18  4:39 ` Darrick J. Wong
@ 2020-01-20 14:29   ` Eric Sandeen
  2020-01-21 18:37     ` Darrick J. Wong
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Sandeen @ 2020-01-20 14:29 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On 1/17/20 10:39 PM, Darrick J. Wong wrote:
> On Fri, Jan 17, 2020 at 05:17:11PM -0600, Eric Sandeen wrote:
>> The ->data_entry_p() op went away in v5.5 kernelspace, so rework
>> xfs_repair to use ->data_entry_offset instead, in preparation
>> for the v5.5 libxfs backport.
>>
>> This could later be cleaned up to use offsets as was done
>> in kernel commit 8073af5153c for example.
> 
> See, now that you've said that, I start wondering why not do that?

Because this is the fast/safe path to getting the libxfs merge done IMHO ;)

...


>> @@ -1834,7 +1834,7 @@ longform_dir2_entry_check_data(
>>  			       (dep->name[0] == '.' && dep->namelen == 1));
>>  			add_inode_ref(current_irec, current_ino_offset);
>>  			if (da_bno != 0 ||
>> -			    dep != M_DIROPS(mp)->data_entry_p(d)) {
>> +			    dep != (void *)d + M_DIROPS(mp)->data_entry_offset) {
> 
> Er.... void pointer arithmetic?

er, let me take another look at that.

-eric



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xfs_repair: stop using ->data_entry_p()
  2020-01-20 14:29   ` Eric Sandeen
@ 2020-01-21 18:37     ` Darrick J. Wong
  2020-01-22  0:41       ` Darrick J. Wong
  0 siblings, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2020-01-21 18:37 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: linux-xfs

On Mon, Jan 20, 2020 at 08:29:17AM -0600, Eric Sandeen wrote:
> On 1/17/20 10:39 PM, Darrick J. Wong wrote:
> > On Fri, Jan 17, 2020 at 05:17:11PM -0600, Eric Sandeen wrote:
> >> The ->data_entry_p() op went away in v5.5 kernelspace, so rework
> >> xfs_repair to use ->data_entry_offset instead, in preparation
> >> for the v5.5 libxfs backport.
> >>
> >> This could later be cleaned up to use offsets as was done
> >> in kernel commit 8073af5153c for example.
> > 
> > See, now that you've said that, I start wondering why not do that?
> 
> Because this is the fast/safe path to getting the libxfs merge done IMHO ;)
> 
> ...
> 
> 
> >> @@ -1834,7 +1834,7 @@ longform_dir2_entry_check_data(
> >>  			       (dep->name[0] == '.' && dep->namelen == 1));
> >>  			add_inode_ref(current_irec, current_ino_offset);
> >>  			if (da_bno != 0 ||
> >> -			    dep != M_DIROPS(mp)->data_entry_p(d)) {
> >> +			    dep != (void *)d + M_DIROPS(mp)->data_entry_offset) {
> > 
> > Er.... void pointer arithmetic?
> 
> er, let me take another look at that.

fmeh, we apparently allow this gcc extension in the kernel so I guess
it's fine for xfsprogs :P

--D

> -eric
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xfs_repair: stop using ->data_entry_p()
  2020-01-21 18:37     ` Darrick J. Wong
@ 2020-01-22  0:41       ` Darrick J. Wong
  0 siblings, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2020-01-22  0:41 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: linux-xfs

On Tue, Jan 21, 2020 at 10:37:06AM -0800, Darrick J. Wong wrote:
> On Mon, Jan 20, 2020 at 08:29:17AM -0600, Eric Sandeen wrote:
> > On 1/17/20 10:39 PM, Darrick J. Wong wrote:
> > > On Fri, Jan 17, 2020 at 05:17:11PM -0600, Eric Sandeen wrote:
> > >> The ->data_entry_p() op went away in v5.5 kernelspace, so rework
> > >> xfs_repair to use ->data_entry_offset instead, in preparation
> > >> for the v5.5 libxfs backport.
> > >>
> > >> This could later be cleaned up to use offsets as was done
> > >> in kernel commit 8073af5153c for example.
> > > 
> > > See, now that you've said that, I start wondering why not do that?
> > 
> > Because this is the fast/safe path to getting the libxfs merge done IMHO ;)
> > 
> > ...
> > 
> > 
> > >> @@ -1834,7 +1834,7 @@ longform_dir2_entry_check_data(
> > >>  			       (dep->name[0] == '.' && dep->namelen == 1));
> > >>  			add_inode_ref(current_irec, current_ino_offset);
> > >>  			if (da_bno != 0 ||
> > >> -			    dep != M_DIROPS(mp)->data_entry_p(d)) {
> > >> +			    dep != (void *)d + M_DIROPS(mp)->data_entry_offset) {
> > > 
> > > Er.... void pointer arithmetic?
> > 
> > er, let me take another look at that.
> 
> fmeh, we apparently allow this gcc extension in the kernel so I guess
> it's fine for xfsprogs :P
> 

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> --D
> 
> > -eric
> > 
> > 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-01-22  0:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-17 23:17 [PATCH] xfs_repair: stop using ->data_entry_p() Eric Sandeen
2020-01-18  4:39 ` Darrick J. Wong
2020-01-20 14:29   ` Eric Sandeen
2020-01-21 18:37     ` Darrick J. Wong
2020-01-22  0:41       ` Darrick J. Wong
2020-01-20  8:58 ` Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.