Linux-ext4 Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/1] ext4: Fix bs < ps issue reported with dioread_nolock  mount opt
@ 2020-10-08 15:02 Ritesh Harjani
  2020-10-08 16:04 ` Jan Kara
  2020-10-09  6:46 ` Sedat Dilek
  0 siblings, 2 replies; 7+ messages in thread
From: Ritesh Harjani @ 2020-10-08 15:02 UTC (permalink / raw)
  To: linux-ext4
  Cc: linux-fsdevel, tytso, jack, anju, Ritesh Harjani, Aneesh Kumar K . V

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 <aneesh.kumar@linux.ibm.com>
Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
---
 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


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

* Re: [PATCH 1/1] ext4: Fix bs < ps issue reported with dioread_nolock mount opt
  2020-10-08 15:02 [PATCH 1/1] ext4: Fix bs < ps issue reported with dioread_nolock mount opt Ritesh Harjani
@ 2020-10-08 16:04 ` Jan Kara
  2020-10-09  6:46 ` Sedat Dilek
  1 sibling, 0 replies; 7+ messages in thread
From: Jan Kara @ 2020-10-08 16:04 UTC (permalink / raw)
  To: Ritesh Harjani
  Cc: linux-ext4, linux-fsdevel, tytso, jack, anju, Aneesh Kumar K . V

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 <aneesh.kumar@linux.ibm.com>
> Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>

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

Reviewed-by: Jan Kara <jack@suse.cz>

								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 <jack@suse.com>
SUSE Labs, CR

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

* Re: [PATCH 1/1] ext4: Fix bs < ps issue reported with dioread_nolock mount opt
  2020-10-08 15:02 [PATCH 1/1] ext4: Fix bs < ps issue reported with dioread_nolock mount opt Ritesh Harjani
  2020-10-08 16:04 ` Jan Kara
@ 2020-10-09  6:46 ` Sedat Dilek
  2020-10-09  7:18   ` Ritesh Harjani
  1 sibling, 1 reply; 7+ messages in thread
From: Sedat Dilek @ 2020-10-09  6:46 UTC (permalink / raw)
  To: Ritesh Harjani
  Cc: linux-ext4, linux-fsdevel, tytso, jack, anju, Aneesh Kumar K . V

On Thu, Oct 8, 2020 at 5:56 PM Ritesh Harjani <riteshh@linux.ibm.com> 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 <aneesh.kumar@linux.ibm.com>
> Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> ---
>  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
>

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

* Re: [PATCH 1/1] ext4: Fix bs < ps issue reported with dioread_nolock mount opt
  2020-10-09  6:46 ` Sedat Dilek
@ 2020-10-09  7:18   ` Ritesh Harjani
  2020-10-09 10:18     ` Sedat Dilek
  0 siblings, 1 reply; 7+ messages in thread
From: Ritesh Harjani @ 2020-10-09  7:18 UTC (permalink / raw)
  To: sedat.dilek
  Cc: linux-ext4, linux-fsdevel, tytso, jack, anju, Aneesh Kumar K . V



On 10/9/20 12:16 PM, Sedat Dilek wrote:
> On Thu, Oct 8, 2020 at 5:56 PM Ritesh Harjani <riteshh@linux.ibm.com> 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

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

* Re: [PATCH 1/1] ext4: Fix bs < ps issue reported with dioread_nolock mount opt
  2020-10-09  7:18   ` Ritesh Harjani
@ 2020-10-09 10:18     ` Sedat Dilek
  2020-10-09 10:28       ` Sedat Dilek
  2020-10-09 15:58       ` Theodore Y. Ts'o
  0 siblings, 2 replies; 7+ messages in thread
From: Sedat Dilek @ 2020-10-09 10:18 UTC (permalink / raw)
  To: Ritesh Harjani
  Cc: linux-ext4, linux-fsdevel, tytso, jack, anju, Aneesh Kumar K . V

On Fri, Oct 9, 2020 at 9:19 AM Ritesh Harjani <riteshh@linux.ibm.com> wrote:
>
>
>
> On 10/9/20 12:16 PM, Sedat Dilek wrote:
> > On Thu, Oct 8, 2020 at 5:56 PM Ritesh Harjani <riteshh@linux.ibm.com> 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 -

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

* Re: [PATCH 1/1] ext4: Fix bs < ps issue reported with dioread_nolock mount opt
  2020-10-09 10:18     ` Sedat Dilek
@ 2020-10-09 10:28       ` Sedat Dilek
  2020-10-09 15:58       ` Theodore Y. Ts'o
  1 sibling, 0 replies; 7+ messages in thread
From: Sedat Dilek @ 2020-10-09 10:28 UTC (permalink / raw)
  To: Ritesh Harjani
  Cc: linux-ext4, linux-fsdevel, tytso, jack, anju, Aneesh Kumar K . V

On Fri, Oct 9, 2020 at 12:18 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Fri, Oct 9, 2020 at 9:19 AM Ritesh Harjani <riteshh@linux.ibm.com> wrote:
> >
> >
> >
> > On 10/9/20 12:16 PM, Sedat Dilek wrote:
> > > On Thu, Oct 8, 2020 at 5:56 PM Ritesh Harjani <riteshh@linux.ibm.com> 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 -

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

* Re: [PATCH 1/1] ext4: Fix bs < ps issue reported with dioread_nolock mount opt
  2020-10-09 10:18     ` Sedat Dilek
  2020-10-09 10:28       ` Sedat Dilek
@ 2020-10-09 15:58       ` Theodore Y. Ts'o
  1 sibling, 0 replies; 7+ messages in thread
From: Theodore Y. Ts'o @ 2020-10-09 15:58 UTC (permalink / raw)
  To: Sedat Dilek
  Cc: Ritesh Harjani, linux-ext4, linux-fsdevel, jack, anju,
	Aneesh Kumar K . V

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

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

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-08 15:02 [PATCH 1/1] ext4: Fix bs < ps issue reported with dioread_nolock mount opt Ritesh Harjani
2020-10-08 16:04 ` Jan Kara
2020-10-09  6:46 ` Sedat Dilek
2020-10-09  7:18   ` Ritesh Harjani
2020-10-09 10:18     ` Sedat Dilek
2020-10-09 10:28       ` Sedat Dilek
2020-10-09 15:58       ` Theodore Y. Ts'o

Linux-ext4 Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-ext4/0 linux-ext4/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-ext4 linux-ext4/ https://lore.kernel.org/linux-ext4 \
		linux-ext4@vger.kernel.org
	public-inbox-index linux-ext4

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-ext4


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git