* [PATCH] btrfs: Use raw numbers when setting passing uptodate parameter to btrfs_writepage_endio_finish_ordered
@ 2019-03-20 19:53 Nikolay Borisov
2019-03-21 14:30 ` David Sterba
0 siblings, 1 reply; 5+ messages in thread
From: Nikolay Borisov @ 2019-03-20 19:53 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
The uptodate parameter of btrfs_writepage_endio_finish_ordered is used
to signal whether an error has occured while writing the given page.
0 signal an error, which is propagated to callees and 1 signifies
success. In end_compressed_bio_write the ->bi_status is checked and
based on it either BLK_STS_OK (0) or BLK_STS_NOTSUPP (1) are used. While
from functional point of view this is ok it's a for the poor reader of
the code, since the block layer values are conflated with the semantics
of the parameter.
Just use plain 0 or 1. No functional changes.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
fs/btrfs/compression.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index eb8e20b740d6..f82441f18512 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -250,7 +250,7 @@ static void end_compressed_bio_write(struct bio *bio)
cb->compressed_pages[0]->mapping = cb->inode->i_mapping;
btrfs_writepage_endio_finish_ordered(cb->compressed_pages[0],
cb->start, cb->start + cb->len - 1,
- bio->bi_status ? BLK_STS_OK : BLK_STS_NOTSUPP);
+ bio->bi_status ? 0 : 1);
cb->compressed_pages[0]->mapping = NULL;
end_compressed_writeback(inode, cb);
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs: Use raw numbers when setting passing uptodate parameter to btrfs_writepage_endio_finish_ordered
2019-03-20 19:53 [PATCH] btrfs: Use raw numbers when setting passing uptodate parameter to btrfs_writepage_endio_finish_ordered Nikolay Borisov
@ 2019-03-21 14:30 ` David Sterba
2019-03-21 14:31 ` Nikolay Borisov
0 siblings, 1 reply; 5+ messages in thread
From: David Sterba @ 2019-03-21 14:30 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: linux-btrfs, anand.jain
On Wed, Mar 20, 2019 at 09:53:16PM +0200, Nikolay Borisov wrote:
> The uptodate parameter of btrfs_writepage_endio_finish_ordered is used
> to signal whether an error has occured while writing the given page.
> 0 signal an error, which is propagated to callees and 1 signifies
> success. In end_compressed_bio_write the ->bi_status is checked and
> based on it either BLK_STS_OK (0) or BLK_STS_NOTSUPP (1) are used. While
> from functional point of view this is ok it's a for the poor reader of
> the code, since the block layer values are conflated with the semantics
> of the parameter.
>
> Just use plain 0 or 1. No functional changes.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/compression.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index eb8e20b740d6..f82441f18512 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -250,7 +250,7 @@ static void end_compressed_bio_write(struct bio *bio)
> cb->compressed_pages[0]->mapping = cb->inode->i_mapping;
> btrfs_writepage_endio_finish_ordered(cb->compressed_pages[0],
> cb->start, cb->start + cb->len - 1,
> - bio->bi_status ? BLK_STS_OK : BLK_STS_NOTSUPP);
> + bio->bi_status ? 0 : 1);
Essentially reverting 2dbe0c77186c691386b74391039899808a4d3f59, but
using the BLK_STS constants is definetelly wrong. The values match the
intended use of 'uptodate', but otherwise are confusing.
I actually think it can be simplified to bio->bi_staus == BLK_STS_OK.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs: Use raw numbers when setting passing uptodate parameter to btrfs_writepage_endio_finish_ordered
2019-03-21 14:30 ` David Sterba
@ 2019-03-21 14:31 ` Nikolay Borisov
2019-03-21 15:25 ` David Sterba
0 siblings, 1 reply; 5+ messages in thread
From: Nikolay Borisov @ 2019-03-21 14:31 UTC (permalink / raw)
To: dsterba, linux-btrfs, anand.jain
On 21.03.19 г. 16:30 ч., David Sterba wrote:
> On Wed, Mar 20, 2019 at 09:53:16PM +0200, Nikolay Borisov wrote:
>> The uptodate parameter of btrfs_writepage_endio_finish_ordered is used
>> to signal whether an error has occured while writing the given page.
>> 0 signal an error, which is propagated to callees and 1 signifies
>> success. In end_compressed_bio_write the ->bi_status is checked and
>> based on it either BLK_STS_OK (0) or BLK_STS_NOTSUPP (1) are used. While
>> from functional point of view this is ok it's a for the poor reader of
>> the code, since the block layer values are conflated with the semantics
>> of the parameter.
>>
>> Just use plain 0 or 1. No functional changes.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>> fs/btrfs/compression.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
>> index eb8e20b740d6..f82441f18512 100644
>> --- a/fs/btrfs/compression.c
>> +++ b/fs/btrfs/compression.c
>> @@ -250,7 +250,7 @@ static void end_compressed_bio_write(struct bio *bio)
>> cb->compressed_pages[0]->mapping = cb->inode->i_mapping;
>> btrfs_writepage_endio_finish_ordered(cb->compressed_pages[0],
>> cb->start, cb->start + cb->len - 1,
>> - bio->bi_status ? BLK_STS_OK : BLK_STS_NOTSUPP);
>> + bio->bi_status ? 0 : 1);
>
> Essentially reverting 2dbe0c77186c691386b74391039899808a4d3f59, but
> using the BLK_STS constants is definetelly wrong. The values match the
> intended use of 'uptodate', but otherwise are confusing.
>
> I actually think it can be simplified to bio->bi_staus == BLK_STS_OK.
Yes that would work, are you going to fix up the patch or shall I resend?
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs: Use raw numbers when setting passing uptodate parameter to btrfs_writepage_endio_finish_ordered
2019-03-21 14:31 ` Nikolay Borisov
@ 2019-03-21 15:25 ` David Sterba
2019-03-22 13:51 ` Anand Jain
0 siblings, 1 reply; 5+ messages in thread
From: David Sterba @ 2019-03-21 15:25 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: dsterba, linux-btrfs, anand.jain
On Thu, Mar 21, 2019 at 04:31:53PM +0200, Nikolay Borisov wrote:
>
>
> On 21.03.19 г. 16:30 ч., David Sterba wrote:
> > On Wed, Mar 20, 2019 at 09:53:16PM +0200, Nikolay Borisov wrote:
> >> The uptodate parameter of btrfs_writepage_endio_finish_ordered is used
> >> to signal whether an error has occured while writing the given page.
> >> 0 signal an error, which is propagated to callees and 1 signifies
> >> success. In end_compressed_bio_write the ->bi_status is checked and
> >> based on it either BLK_STS_OK (0) or BLK_STS_NOTSUPP (1) are used. While
> >> from functional point of view this is ok it's a for the poor reader of
> >> the code, since the block layer values are conflated with the semantics
> >> of the parameter.
> >>
> >> Just use plain 0 or 1. No functional changes.
> >>
> >> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> >> ---
> >> fs/btrfs/compression.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> >> index eb8e20b740d6..f82441f18512 100644
> >> --- a/fs/btrfs/compression.c
> >> +++ b/fs/btrfs/compression.c
> >> @@ -250,7 +250,7 @@ static void end_compressed_bio_write(struct bio *bio)
> >> cb->compressed_pages[0]->mapping = cb->inode->i_mapping;
> >> btrfs_writepage_endio_finish_ordered(cb->compressed_pages[0],
> >> cb->start, cb->start + cb->len - 1,
> >> - bio->bi_status ? BLK_STS_OK : BLK_STS_NOTSUPP);
> >> + bio->bi_status ? 0 : 1);
> >
> > Essentially reverting 2dbe0c77186c691386b74391039899808a4d3f59, but
> > using the BLK_STS constants is definetelly wrong. The values match the
> > intended use of 'uptodate', but otherwise are confusing.
> >
> > I actually think it can be simplified to bio->bi_staus == BLK_STS_OK.
>
> Yes that would work, are you going to fix up the patch or shall I resend?
I'll fix it.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs: Use raw numbers when setting passing uptodate parameter to btrfs_writepage_endio_finish_ordered
2019-03-21 15:25 ` David Sterba
@ 2019-03-22 13:51 ` Anand Jain
0 siblings, 0 replies; 5+ messages in thread
From: Anand Jain @ 2019-03-22 13:51 UTC (permalink / raw)
To: dsterba, Nikolay Borisov, linux-btrfs
On 3/21/19 11:25 PM, David Sterba wrote:
> On Thu, Mar 21, 2019 at 04:31:53PM +0200, Nikolay Borisov wrote:
>>
>>
>> On 21.03.19 г. 16:30 ч., David Sterba wrote:
>>> On Wed, Mar 20, 2019 at 09:53:16PM +0200, Nikolay Borisov wrote:
>>>> The uptodate parameter of btrfs_writepage_endio_finish_ordered is used
>>>> to signal whether an error has occured while writing the given page.
>>>> 0 signal an error, which is propagated to callees and 1 signifies
>>>> success. In end_compressed_bio_write the ->bi_status is checked and
>>>> based on it either BLK_STS_OK (0) or BLK_STS_NOTSUPP (1) are used. While
>>>> from functional point of view this is ok it's a for the poor reader of
>>>> the code, since the block layer values are conflated with the semantics
>>>> of the parameter.
>>>>
>>>> Just use plain 0 or 1. No functional changes.
>>>>
>>>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>>>> ---
>>>> fs/btrfs/compression.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
>>>> index eb8e20b740d6..f82441f18512 100644
>>>> --- a/fs/btrfs/compression.c
>>>> +++ b/fs/btrfs/compression.c
>>>> @@ -250,7 +250,7 @@ static void end_compressed_bio_write(struct bio *bio)
>>>> cb->compressed_pages[0]->mapping = cb->inode->i_mapping;
>>>> btrfs_writepage_endio_finish_ordered(cb->compressed_pages[0],
>>>> cb->start, cb->start + cb->len - 1,
>>>> - bio->bi_status ? BLK_STS_OK : BLK_STS_NOTSUPP);
>>>> + bio->bi_status ? 0 : 1);
>>>
>>> Essentially reverting 2dbe0c77186c691386b74391039899808a4d3f59, but
>>> using the BLK_STS constants is definetelly wrong. The values match the
>>> intended use of 'uptodate', but otherwise are confusing.
>>>
>>> I actually think it can be simplified to bio->bi_staus == BLK_STS_OK.
>>
>> Yes that would work, are you going to fix up the patch or shall I resend?
>
> I'll fix it.
bio->bi_staus == BLK_STS_OK ? 1 : 0);
Is better. Its confusing. Thanks for the fix.
Reviewed-by: Anand Jain <anand.jain@oracle.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-03-22 13:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-20 19:53 [PATCH] btrfs: Use raw numbers when setting passing uptodate parameter to btrfs_writepage_endio_finish_ordered Nikolay Borisov
2019-03-21 14:30 ` David Sterba
2019-03-21 14:31 ` Nikolay Borisov
2019-03-21 15:25 ` David Sterba
2019-03-22 13:51 ` Anand Jain
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.