diff mbox

btrfs: suppress a build warning on building 32bit kernel

Message ID 549BD725.8040603@jp.fujitsu.com (mailing list archive)
State Under Review
Headers show

Commit Message

Satoru Takeuchi Dec. 25, 2014, 9:21 a.m. UTC 
From: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>

Suppress the following warning displayed on building 32bit (i686) kernel.

Comments

David Sterba Dec. 29, 2014, 3:09 p.m. UTC | #1 
On Thu, Dec 25, 2014 at 06:21:41PM +0900, Satoru Takeuchi wrote:
> From: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -2190,7 +2190,7 @@ void btrfs_free_io_failure_record(struct inode *inode, u64 start, u64 end)
>   
>   		next = next_state(state);
>   
> -		failrec = (struct io_failure_record *)state->private;
> +		failrec = (struct io_failure_record *)(unsigned long)state->private;

We're always using the 'private' data to store a pointer to
'struct io_failure_record *', please change the defintion in
'struct extent_state' instead of the typecasting.

>   		free_extent_state(state);
>   		kfree(failrec);
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Satoru Takeuchi Jan. 5, 2015, 8:03 a.m. UTC | #2 
Hi David,

On 2014/12/30 0:09, David Sterba wrote:
> On Thu, Dec 25, 2014 at 06:21:41PM +0900, Satoru Takeuchi wrote:
>> From: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
>> --- a/fs/btrfs/extent_io.c
>> +++ b/fs/btrfs/extent_io.c
>> @@ -2190,7 +2190,7 @@ void btrfs_free_io_failure_record(struct inode *inode, u64 start, u64 end)
>>
>>    		next = next_state(state);
>>
>> -		failrec = (struct io_failure_record *)state->private;
>> +		failrec = (struct io_failure_record *)(unsigned long)state->private;
>
> We're always using the 'private' data to store a pointer to
> 'struct io_failure_record *', please change the defintion in
> 'struct extent_state' instead of the typecasting.

Current definition is as follow.

===============================================================================
struct extent_state {
...
         /* for use by the FS */
         u64 private;
};
===============================================================================

It it OK to changing "u64 private" to "struct io_failure_record *failrec"
and change "{set,get}_state_private()" to "{set,get}_state_failrec()?
Or is it better to keep the name "private" as is and just change its type
to "unsigned long" or "(void *)"?

Thanks,
Satoru

>
>>    		free_extent_state(state);
>>    		kfree(failrec);

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba Jan. 6, 2015, 2:07 p.m. UTC | #3 
On Mon, Jan 05, 2015 at 05:03:29PM +0900, Satoru Takeuchi wrote:
> >> -		failrec = (struct io_failure_record *)state->private;
> >> +		failrec = (struct io_failure_record *)(unsigned long)state->private;
> >
> > We're always using the 'private' data to store a pointer to
> > 'struct io_failure_record *', please change the defintion in
> > 'struct extent_state' instead of the typecasting.
> 
> Current definition is as follow.
> 
> ===============================================================================
> struct extent_state {
> ...
>          /* for use by the FS */
>          u64 private;
> };
> ===============================================================================
> 
> It it OK to changing "u64 private" to "struct io_failure_record *failrec"
> and change "{set,get}_state_private()" to "{set,get}_state_failrec()?
> Or is it better to keep the name "private" as is and just change its type
> to "unsigned long" or "(void *)"?

I've looked at the implied changes that set/get functions renaming would
need, also to keep the code sane. It does not seem to be small enough to
fold in this patch so please go on with adding the typecasts. The code
could use some cleanups but bugfixes first.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

===============================================================================
...
   CC [M]  fs/btrfs/extent_io.o
fs/btrfs/extent_io.c: In function ‘btrfs_free_io_failure_record’:
fs/btrfs/extent_io.c:2193:13: warning: cast to pointer from integer of
different size [-Wint-to-pointer-cast]
    failrec = (struct io_failure_record *)state->private;
...
===============================================================================

Signed-off-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
Reported-by: Chris Murphy <chris@colorremedies.com>

---
  fs/btrfs/extent_io.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 4ebabd2..790dbae 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2190,7 +2190,7 @@  void btrfs_free_io_failure_record(struct inode *inode, u64 start, u64 end)
  
  		next = next_state(state);
  
-		failrec = (struct io_failure_record *)state->private;
+		failrec = (struct io_failure_record *)(unsigned long)state->private;
  		free_extent_state(state);
  		kfree(failrec);