* [PATCH] direct-io: allow direct writes to empty inodes
@ 2018-10-08 23:58 Ernesto A. Fernández
2018-10-26 9:30 ` Jan Kara
0 siblings, 1 reply; 8+ messages in thread
From: Ernesto A. Fernández @ 2018-10-08 23:58 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Alexander Viro
On a DIO_SKIP_HOLES filesystem, the ->get_block() method is currently
not allowed to create blocks for an empty inode. This confusion comes
from trying to bit shift a negative number, so check the size of the
inode first.
The problem is most visible for hfsplus, because the fallback to
buffered I/O doesn't happen and the write fails with EIO. This is in
part the fault of the module, because it gives a wrong return value on
->get_block(); that will be fixed in a separate patch.
Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com>
---
fs/direct-io.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/direct-io.c b/fs/direct-io.c
index 093fb54cd316..9a7b91a3b7a7 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -679,6 +679,7 @@ static int get_more_blocks(struct dio *dio, struct dio_submit *sdio,
unsigned long fs_count; /* Number of filesystem-sized blocks */
int create;
unsigned int i_blkbits = sdio->blkbits + sdio->blkfactor;
+ loff_t i_size;
/*
* If there was a memory error and we've overwritten all the
@@ -708,8 +709,8 @@ static int get_more_blocks(struct dio *dio, struct dio_submit *sdio,
*/
create = dio->op == REQ_OP_WRITE;
if (dio->flags & DIO_SKIP_HOLES) {
- if (fs_startblk <= ((i_size_read(dio->inode) - 1) >>
- i_blkbits))
+ i_size = i_size_read(dio->inode);
+ if (i_size && fs_startblk <= (i_size - 1) >> i_blkbits)
create = 0;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] direct-io: allow direct writes to empty inodes
2018-10-08 23:58 [PATCH] direct-io: allow direct writes to empty inodes Ernesto A. Fernández
@ 2018-10-26 9:30 ` Jan Kara
2018-10-26 14:29 ` Jens Axboe
0 siblings, 1 reply; 8+ messages in thread
From: Jan Kara @ 2018-10-26 9:30 UTC (permalink / raw)
To: Ernesto A. Fernández; +Cc: linux-fsdevel, Alexander Viro, Jens Axboe
On Mon 08-10-18 20:58:23, Ernesto A. Fern�ndez wrote:
> On a DIO_SKIP_HOLES filesystem, the ->get_block() method is currently
> not allowed to create blocks for an empty inode. This confusion comes
> from trying to bit shift a negative number, so check the size of the
> inode first.
>
> The problem is most visible for hfsplus, because the fallback to
> buffered I/O doesn't happen and the write fails with EIO. This is in
> part the fault of the module, because it gives a wrong return value on
> ->get_block(); that will be fixed in a separate patch.
>
> Signed-off-by: Ernesto A. Fern�ndez <ernesto.mnd.fernandez@gmail.com>
Good catch. The patch looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
Also Jens often picks up patches for direct IO code so added him to CC.
Jens?
Honza
> ---
> fs/direct-io.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fs/direct-io.c b/fs/direct-io.c
> index 093fb54cd316..9a7b91a3b7a7 100644
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -679,6 +679,7 @@ static int get_more_blocks(struct dio *dio, struct dio_submit *sdio,
> unsigned long fs_count; /* Number of filesystem-sized blocks */
> int create;
> unsigned int i_blkbits = sdio->blkbits + sdio->blkfactor;
> + loff_t i_size;
>
> /*
> * If there was a memory error and we've overwritten all the
> @@ -708,8 +709,8 @@ static int get_more_blocks(struct dio *dio, struct dio_submit *sdio,
> */
> create = dio->op == REQ_OP_WRITE;
> if (dio->flags & DIO_SKIP_HOLES) {
> - if (fs_startblk <= ((i_size_read(dio->inode) - 1) >>
> - i_blkbits))
> + i_size = i_size_read(dio->inode);
> + if (i_size && fs_startblk <= (i_size - 1) >> i_blkbits)
> create = 0;
> }
>
> --
> 2.11.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] direct-io: allow direct writes to empty inodes
2018-10-26 9:30 ` Jan Kara
@ 2018-10-26 14:29 ` Jens Axboe
2018-10-27 4:12 ` Ernesto A. Fernández
0 siblings, 1 reply; 8+ messages in thread
From: Jens Axboe @ 2018-10-26 14:29 UTC (permalink / raw)
To: Jan Kara, Ernesto A. Fernández; +Cc: linux-fsdevel, Alexander Viro
On 10/26/18 3:30 AM, Jan Kara wrote:
> On Mon 08-10-18 20:58:23, Ernesto A. Fernández wrote:
>> On a DIO_SKIP_HOLES filesystem, the ->get_block() method is currently
>> not allowed to create blocks for an empty inode. This confusion comes
>> from trying to bit shift a negative number, so check the size of the
>> inode first.
>>
>> The problem is most visible for hfsplus, because the fallback to
>> buffered I/O doesn't happen and the write fails with EIO. This is in
>> part the fault of the module, because it gives a wrong return value on
>> ->get_block(); that will be fixed in a separate patch.
>>
>> Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com>
>
> Good catch. The patch looks good. You can add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>
>
> Also Jens often picks up patches for direct IO code so added him to CC.
> Jens?
Looks good to me. Ernesto, did you run this through xfstests as well?
--
Jens Axboe
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] direct-io: allow direct writes to empty inodes
2018-10-26 14:29 ` Jens Axboe
@ 2018-10-27 4:12 ` Ernesto A. Fernández
2018-10-27 16:54 ` Jens Axboe
0 siblings, 1 reply; 8+ messages in thread
From: Ernesto A. Fernández @ 2018-10-27 4:12 UTC (permalink / raw)
To: Jens Axboe; +Cc: Jan Kara, linux-fsdevel, Alexander Viro
On Fri, Oct 26, 2018 at 08:29:00AM -0600, Jens Axboe wrote:
> On 10/26/18 3:30 AM, Jan Kara wrote:
> > On Mon 08-10-18 20:58:23, Ernesto A. Fernández wrote:
> >> On a DIO_SKIP_HOLES filesystem, the ->get_block() method is currently
> >> not allowed to create blocks for an empty inode. This confusion comes
> >> from trying to bit shift a negative number, so check the size of the
> >> inode first.
> >>
> >> The problem is most visible for hfsplus, because the fallback to
> >> buffered I/O doesn't happen and the write fails with EIO. This is in
> >> part the fault of the module, because it gives a wrong return value on
> >> ->get_block(); that will be fixed in a separate patch.
> >>
> >> Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com>
> >
> > Good catch. The patch looks good. You can add:
> >
> > Reviewed-by: Jan Kara <jack@suse.cz>
> >
> > Also Jens often picks up patches for direct IO code so added him to CC.
> > Jens?
>
> Looks good to me. Ernesto, did you run this through xfstests as well?
I only ran the quick tests for ext2. Also for hfsplus, but those are not
very meaningful because too many are failing.
>
> --
> Jens Axboe
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] direct-io: allow direct writes to empty inodes
2018-10-27 4:12 ` Ernesto A. Fernández
@ 2018-10-27 16:54 ` Jens Axboe
2019-01-07 18:03 ` Ernesto A. Fernández
0 siblings, 1 reply; 8+ messages in thread
From: Jens Axboe @ 2018-10-27 16:54 UTC (permalink / raw)
To: Ernesto A. Fernández; +Cc: Jan Kara, linux-fsdevel, Alexander Viro
On 10/26/18 10:12 PM, Ernesto A. Fernández wrote:
> On Fri, Oct 26, 2018 at 08:29:00AM -0600, Jens Axboe wrote:
>> On 10/26/18 3:30 AM, Jan Kara wrote:
>>> On Mon 08-10-18 20:58:23, Ernesto A. Fernández wrote:
>>>> On a DIO_SKIP_HOLES filesystem, the ->get_block() method is currently
>>>> not allowed to create blocks for an empty inode. This confusion comes
>>>> from trying to bit shift a negative number, so check the size of the
>>>> inode first.
>>>>
>>>> The problem is most visible for hfsplus, because the fallback to
>>>> buffered I/O doesn't happen and the write fails with EIO. This is in
>>>> part the fault of the module, because it gives a wrong return value on
>>>> ->get_block(); that will be fixed in a separate patch.
>>>>
>>>> Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com>
>>>
>>> Good catch. The patch looks good. You can add:
>>>
>>> Reviewed-by: Jan Kara <jack@suse.cz>
>>>
>>> Also Jens often picks up patches for direct IO code so added him to CC.
>>> Jens?
>>
>> Looks good to me. Ernesto, did you run this through xfstests as well?
>
> I only ran the quick tests for ext2. Also for hfsplus, but those are not
> very meaningful because too many are failing.
OK, that's useful. I'll run it through the whole thing just in case,
but I don't expect to find anything.
--
Jens Axboe
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] direct-io: allow direct writes to empty inodes
2018-10-27 16:54 ` Jens Axboe
@ 2019-01-07 18:03 ` Ernesto A. Fernández
2019-01-22 0:04 ` Ernesto A. Fernández
0 siblings, 1 reply; 8+ messages in thread
From: Ernesto A. Fernández @ 2019-01-07 18:03 UTC (permalink / raw)
To: Jens Axboe; +Cc: Jan Kara, linux-fsdevel, Alexander Viro, Jeff Moyer
On Sat, Oct 27, 2018 at 10:54:09AM -0600, Jens Axboe wrote:
> On 10/26/18 10:12 PM, Ernesto A. Fernández wrote:
> > On Fri, Oct 26, 2018 at 08:29:00AM -0600, Jens Axboe wrote:
> >> On 10/26/18 3:30 AM, Jan Kara wrote:
> >>> On Mon 08-10-18 20:58:23, Ernesto A. Fernández wrote:
> >>>> On a DIO_SKIP_HOLES filesystem, the ->get_block() method is currently
> >>>> not allowed to create blocks for an empty inode. This confusion comes
> >>>> from trying to bit shift a negative number, so check the size of the
> >>>> inode first.
> >>>>
> >>>> The problem is most visible for hfsplus, because the fallback to
> >>>> buffered I/O doesn't happen and the write fails with EIO. This is in
> >>>> part the fault of the module, because it gives a wrong return value on
> >>>> ->get_block(); that will be fixed in a separate patch.
> >>>>
> >>>> Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com>
> >>>
> >>> Good catch. The patch looks good. You can add:
> >>>
> >>> Reviewed-by: Jan Kara <jack@suse.cz>
> >>>
> >>> Also Jens often picks up patches for direct IO code so added him to CC.
> >>> Jens?
> >>
> >> Looks good to me. Ernesto, did you run this through xfstests as well?
> >
> > I only ran the quick tests for ext2. Also for hfsplus, but those are not
> > very meaningful because too many are failing.
>
> OK, that's useful. I'll run it through the whole thing just in case,
> but I don't expect to find anything.
So what happened with this patch? Any issues?
>
> --
> Jens Axboe
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] direct-io: allow direct writes to empty inodes
2019-01-07 18:03 ` Ernesto A. Fernández
@ 2019-01-22 0:04 ` Ernesto A. Fernández
2019-01-22 15:27 ` Jens Axboe
0 siblings, 1 reply; 8+ messages in thread
From: Ernesto A. Fernández @ 2019-01-22 0:04 UTC (permalink / raw)
To: Jens Axboe; +Cc: Jan Kara, linux-fsdevel, Alexander Viro, Jeff Moyer
On Mon, Jan 07, 2019 at 03:03:08PM -0300, Ernesto A. Fernández wrote:
> On Sat, Oct 27, 2018 at 10:54:09AM -0600, Jens Axboe wrote:
> > On 10/26/18 10:12 PM, Ernesto A. Fernández wrote:
> > > On Fri, Oct 26, 2018 at 08:29:00AM -0600, Jens Axboe wrote:
> > >> On 10/26/18 3:30 AM, Jan Kara wrote:
> > >>> On Mon 08-10-18 20:58:23, Ernesto A. Fernández wrote:
> > >>>> On a DIO_SKIP_HOLES filesystem, the ->get_block() method is currently
> > >>>> not allowed to create blocks for an empty inode. This confusion comes
> > >>>> from trying to bit shift a negative number, so check the size of the
> > >>>> inode first.
> > >>>>
> > >>>> The problem is most visible for hfsplus, because the fallback to
> > >>>> buffered I/O doesn't happen and the write fails with EIO. This is in
> > >>>> part the fault of the module, because it gives a wrong return value on
> > >>>> ->get_block(); that will be fixed in a separate patch.
> > >>>>
> > >>>> Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com>
> > >>>
> > >>> Good catch. The patch looks good. You can add:
> > >>>
> > >>> Reviewed-by: Jan Kara <jack@suse.cz>
> > >>>
> > >>> Also Jens often picks up patches for direct IO code so added him to CC.
> > >>> Jens?
> > >>
> > >> Looks good to me. Ernesto, did you run this through xfstests as well?
> > >
> > > I only ran the quick tests for ext2. Also for hfsplus, but those are not
> > > very meaningful because too many are failing.
> >
> > OK, that's useful. I'll run it through the whole thing just in case,
> > but I don't expect to find anything.
>
> So what happened with this patch? Any issues?
Ping?
>
> >
> > --
> > Jens Axboe
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] direct-io: allow direct writes to empty inodes
2019-01-22 0:04 ` Ernesto A. Fernández
@ 2019-01-22 15:27 ` Jens Axboe
0 siblings, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2019-01-22 15:27 UTC (permalink / raw)
To: Ernesto A. Fernández
Cc: Jan Kara, linux-fsdevel, Alexander Viro, Jeff Moyer
On 1/21/19 5:04 PM, Ernesto A. Fernández wrote:
> On Mon, Jan 07, 2019 at 03:03:08PM -0300, Ernesto A. Fernández wrote:
>> On Sat, Oct 27, 2018 at 10:54:09AM -0600, Jens Axboe wrote:
>>> On 10/26/18 10:12 PM, Ernesto A. Fernández wrote:
>>>> On Fri, Oct 26, 2018 at 08:29:00AM -0600, Jens Axboe wrote:
>>>>> On 10/26/18 3:30 AM, Jan Kara wrote:
>>>>>> On Mon 08-10-18 20:58:23, Ernesto A. Fernández wrote:
>>>>>>> On a DIO_SKIP_HOLES filesystem, the ->get_block() method is currently
>>>>>>> not allowed to create blocks for an empty inode. This confusion comes
>>>>>>> from trying to bit shift a negative number, so check the size of the
>>>>>>> inode first.
>>>>>>>
>>>>>>> The problem is most visible for hfsplus, because the fallback to
>>>>>>> buffered I/O doesn't happen and the write fails with EIO. This is in
>>>>>>> part the fault of the module, because it gives a wrong return value on
>>>>>>> ->get_block(); that will be fixed in a separate patch.
>>>>>>>
>>>>>>> Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com>
>>>>>>
>>>>>> Good catch. The patch looks good. You can add:
>>>>>>
>>>>>> Reviewed-by: Jan Kara <jack@suse.cz>
>>>>>>
>>>>>> Also Jens often picks up patches for direct IO code so added him to CC.
>>>>>> Jens?
>>>>>
>>>>> Looks good to me. Ernesto, did you run this through xfstests as well?
>>>>
>>>> I only ran the quick tests for ext2. Also for hfsplus, but those are not
>>>> very meaningful because too many are failing.
>>>
>>> OK, that's useful. I'll run it through the whole thing just in case,
>>> but I don't expect to find anything.
>>
>> So what happened with this patch? Any issues?
>
> Ping?
Sorry, I dropped this one on the floor. Now queued up, thanks for the
ping!
--
Jens Axboe
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-01-22 15:27 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-08 23:58 [PATCH] direct-io: allow direct writes to empty inodes Ernesto A. Fernández
2018-10-26 9:30 ` Jan Kara
2018-10-26 14:29 ` Jens Axboe
2018-10-27 4:12 ` Ernesto A. Fernández
2018-10-27 16:54 ` Jens Axboe
2019-01-07 18:03 ` Ernesto A. Fernández
2019-01-22 0:04 ` Ernesto A. Fernández
2019-01-22 15:27 ` Jens Axboe
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.