2020-10-08 15:09:06

by Ritesh Harjani

[permalink] [raw]
Subject: [PATCH 1/1] ext4: Fix bs < ps issue reported with dioread_nolock mount opt

left shifting m_lblk by blkbits was causing value overflow and hence
it was not able to convert unwritten to written extent.
So, make sure we typecast it to loff_t before do left shift operation.
Also in func ext4_convert_unwritten_io_end_vec(), make sure to initialize
ret variable to avoid accidentally returning an uninitialized ret.

This patch fixes the issue reported in ext4 for bs < ps with
dioread_nolock mount option.

Fixes: c8cc88163f40df39e50c ("ext4: Add support for blocksize < pagesize in dioread_nolock")
Reported-by: Aneesh Kumar K.V <[email protected]>
Signed-off-by: Ritesh Harjani <[email protected]>
---
fs/ext4/extents.c | 2 +-
fs/ext4/inode.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index a0481582187a..32d610cc896d 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4769,7 +4769,7 @@ int ext4_convert_unwritten_extents(handle_t *handle, struct inode *inode,

int ext4_convert_unwritten_io_end_vec(handle_t *handle, ext4_io_end_t *io_end)
{
- int ret, err = 0;
+ int ret = 0, err = 0;
struct ext4_io_end_vec *io_end_vec;

/*
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index bf596467c234..3021235deaa1 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2254,7 +2254,7 @@ static int mpage_process_page(struct mpage_da_data *mpd, struct page *page,
err = PTR_ERR(io_end_vec);
goto out;
}
- io_end_vec->offset = mpd->map.m_lblk << blkbits;
+ io_end_vec->offset = (loff_t)mpd->map.m_lblk << blkbits;
}
*map_bh = true;
goto out;
--
2.26.2


2020-10-08 16:07:05

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 1/1] ext4: Fix bs < ps issue reported with dioread_nolock mount opt

On Thu 08-10-20 20:32:48, Ritesh Harjani wrote:
> left shifting m_lblk by blkbits was causing value overflow and hence
> it was not able to convert unwritten to written extent.
> So, make sure we typecast it to loff_t before do left shift operation.
> Also in func ext4_convert_unwritten_io_end_vec(), make sure to initialize
> ret variable to avoid accidentally returning an uninitialized ret.
>
> This patch fixes the issue reported in ext4 for bs < ps with
> dioread_nolock mount option.
>
> Fixes: c8cc88163f40df39e50c ("ext4: Add support for blocksize < pagesize in dioread_nolock")
> Reported-by: Aneesh Kumar K.V <[email protected]>
> Signed-off-by: Ritesh Harjani <[email protected]>

Ah, good spotting! The patch looks good. You can add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> fs/ext4/extents.c | 2 +-
> fs/ext4/inode.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index a0481582187a..32d610cc896d 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4769,7 +4769,7 @@ int ext4_convert_unwritten_extents(handle_t *handle, struct inode *inode,
>
> int ext4_convert_unwritten_io_end_vec(handle_t *handle, ext4_io_end_t *io_end)
> {
> - int ret, err = 0;
> + int ret = 0, err = 0;
> struct ext4_io_end_vec *io_end_vec;
>
> /*
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index bf596467c234..3021235deaa1 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -2254,7 +2254,7 @@ static int mpage_process_page(struct mpage_da_data *mpd, struct page *page,
> err = PTR_ERR(io_end_vec);
> goto out;
> }
> - io_end_vec->offset = mpd->map.m_lblk << blkbits;
> + io_end_vec->offset = (loff_t)mpd->map.m_lblk << blkbits;
> }
> *map_bh = true;
> goto out;
> --
> 2.26.2
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2020-10-09 06:47:58

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 1/1] ext4: Fix bs < ps issue reported with dioread_nolock mount opt

On Thu, Oct 8, 2020 at 5:56 PM Ritesh Harjani <[email protected]> wrote:
>
> left shifting m_lblk by blkbits was causing value overflow and hence
> it was not able to convert unwritten to written extent.
> So, make sure we typecast it to loff_t before do left shift operation.
> Also in func ext4_convert_unwritten_io_end_vec(), make sure to initialize
> ret variable to avoid accidentally returning an uninitialized ret.
>
> This patch fixes the issue reported in ext4 for bs < ps with
> dioread_nolock mount option.
>
> Fixes: c8cc88163f40df39e50c ("ext4: Add support for blocksize < pagesize in dioread_nolock")

Fixes: tag should be 12 digits (see [1]).
( Seen while walking through ext-dev Git commits. )

- Sedat -

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst#n183

> Reported-by: Aneesh Kumar K.V <[email protected]>
> Signed-off-by: Ritesh Harjani <[email protected]>
> ---
> fs/ext4/extents.c | 2 +-
> fs/ext4/inode.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index a0481582187a..32d610cc896d 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4769,7 +4769,7 @@ int ext4_convert_unwritten_extents(handle_t *handle, struct inode *inode,
>
> int ext4_convert_unwritten_io_end_vec(handle_t *handle, ext4_io_end_t *io_end)
> {
> - int ret, err = 0;
> + int ret = 0, err = 0;
> struct ext4_io_end_vec *io_end_vec;
>
> /*
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index bf596467c234..3021235deaa1 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -2254,7 +2254,7 @@ static int mpage_process_page(struct mpage_da_data *mpd, struct page *page,
> err = PTR_ERR(io_end_vec);
> goto out;
> }
> - io_end_vec->offset = mpd->map.m_lblk << blkbits;
> + io_end_vec->offset = (loff_t)mpd->map.m_lblk << blkbits;
> }
> *map_bh = true;
> goto out;
> --
> 2.26.2
>

2020-10-09 07:20:33

by Ritesh Harjani

[permalink] [raw]
Subject: Re: [PATCH 1/1] ext4: Fix bs < ps issue reported with dioread_nolock mount opt



On 10/9/20 12:16 PM, Sedat Dilek wrote:
> On Thu, Oct 8, 2020 at 5:56 PM Ritesh Harjani <[email protected]> wrote:
>>
>> left shifting m_lblk by blkbits was causing value overflow and hence
>> it was not able to convert unwritten to written extent.
>> So, make sure we typecast it to loff_t before do left shift operation.
>> Also in func ext4_convert_unwritten_io_end_vec(), make sure to initialize
>> ret variable to avoid accidentally returning an uninitialized ret.
>>
>> This patch fixes the issue reported in ext4 for bs < ps with
>> dioread_nolock mount option.
>>
>> Fixes: c8cc88163f40df39e50c ("ext4: Add support for blocksize < pagesize in dioread_nolock")
>
> Fixes: tag should be 12 digits (see [1]).
> ( Seen while walking through ext-dev Git commits. )


Thanks Sedat, I guess it should be minimum 12 chars [1]

[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst#n177

-ritesh

2020-10-09 12:34:52

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 1/1] ext4: Fix bs < ps issue reported with dioread_nolock mount opt

On Fri, Oct 9, 2020 at 9:19 AM Ritesh Harjani <[email protected]> wrote:
>
>
>
> On 10/9/20 12:16 PM, Sedat Dilek wrote:
> > On Thu, Oct 8, 2020 at 5:56 PM Ritesh Harjani <[email protected]> wrote:
> >>
> >> left shifting m_lblk by blkbits was causing value overflow and hence
> >> it was not able to convert unwritten to written extent.
> >> So, make sure we typecast it to loff_t before do left shift operation.
> >> Also in func ext4_convert_unwritten_io_end_vec(), make sure to initialize
> >> ret variable to avoid accidentally returning an uninitialized ret.
> >>
> >> This patch fixes the issue reported in ext4 for bs < ps with
> >> dioread_nolock mount option.
> >>
> >> Fixes: c8cc88163f40df39e50c ("ext4: Add support for blocksize < pagesize in dioread_nolock")
> >
> > Fixes: tag should be 12 digits (see [1]).
> > ( Seen while walking through ext-dev Git commits. )
>
>
> Thanks Sedat, I guess it should be minimum 12 chars [1]
>
> [1]:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst#n177
>

OK.

In my ~/.gitconfig:

[core]
abbrev = 12

# Check for 'Fixes:' tag used in the Linux-kernel development process
(Thanks Kalle Valo).
# Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst
# Usage: $ git log --format=fixes | head -5
[pretty]
fixes = Fixes: %h (\"%s\")

Hope this is useful for others.

- Sedat -

2020-10-09 12:36:07

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 1/1] ext4: Fix bs < ps issue reported with dioread_nolock mount opt

On Fri, Oct 9, 2020 at 12:18 PM Sedat Dilek <[email protected]> wrote:
>
> On Fri, Oct 9, 2020 at 9:19 AM Ritesh Harjani <[email protected]> wrote:
> >
> >
> >
> > On 10/9/20 12:16 PM, Sedat Dilek wrote:
> > > On Thu, Oct 8, 2020 at 5:56 PM Ritesh Harjani <[email protected]> wrote:
> > >>
> > >> left shifting m_lblk by blkbits was causing value overflow and hence
> > >> it was not able to convert unwritten to written extent.
> > >> So, make sure we typecast it to loff_t before do left shift operation.
> > >> Also in func ext4_convert_unwritten_io_end_vec(), make sure to initialize
> > >> ret variable to avoid accidentally returning an uninitialized ret.
> > >>
> > >> This patch fixes the issue reported in ext4 for bs < ps with
> > >> dioread_nolock mount option.
> > >>
> > >> Fixes: c8cc88163f40df39e50c ("ext4: Add support for blocksize < pagesize in dioread_nolock")
> > >
> > > Fixes: tag should be 12 digits (see [1]).
> > > ( Seen while walking through ext-dev Git commits. )
> >
> >
> > Thanks Sedat, I guess it should be minimum 12 chars [1]
> >
> > [1]:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst#n177
> >
>
> OK.
>
> In my ~/.gitconfig:
>
> [core]
> abbrev = 12
>
> # Check for 'Fixes:' tag used in the Linux-kernel development process
> (Thanks Kalle Valo).
> # Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst
> # Usage: $ git log --format=fixes | head -5
> [pretty]
> fixes = Fixes: %h (\"%s\")
>
> Hope this is useful for others.
>

Changed to...

Link: https://www.kernel.org/doc/html/latest/process/submitting-patches.html

- Sedat -

2020-10-09 16:00:02

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH 1/1] ext4: Fix bs < ps issue reported with dioread_nolock mount opt

On Fri, Oct 09, 2020 at 12:18:23PM +0200, Sedat Dilek wrote:
> > > Fixes: tag should be 12 digits (see [1]).
> > > ( Seen while walking through ext-dev Git commits. )
> >
> > Thanks Sedat, I guess it should be minimum 12 chars [1]

Right, the point is that the commit ID referenced should be at least
12 bytes to avoid ambiguity. There's nothing really wrong with using
more than 12 bytes. I sometimes use 16, myself. It does look like
there is a (mostly harmless) inconsistency between lines 177 and 183
of submitting-patches.rst.

> In my ~/.gitconfig:
>
> [core]
> abbrev = 12
>
> # Check for 'Fixes:' tag used in the Linux-kernel development process
> (Thanks Kalle Valo).2
> # Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst
> # Usage: $ git log --format=fixes | head -5
> [pretty]
> fixes = Fixes: %h (\"%s\")
>
> Hope this is useful for others.

Personally, I find cutting and pasting the full SHA-1 hash and
description, and then cutting down the hash in emacs to be more
convenient, since I generaslly have the git commit from "git log" in
terminal window anyway. But whatever works for each developer. :-)

- Ted