* open sets ext4_da_aops for DAX existing files
@ 2018-09-07 21:23 Kani, Toshi
2018-09-10 12:54 ` Jeff Moyer
2018-09-10 14:29 ` Jan Kara
0 siblings, 2 replies; 9+ messages in thread
From: Kani, Toshi @ 2018-09-07 21:23 UTC (permalink / raw)
To: linux-nvdimm, linux-fsdevel
I noticed that both ext4_da_aops and ext4_dax_aops are used on DAX
mounted ext4 files. Looking at open() path:
New file
--------
lookup_open
ext4_create
__ext4_new_inode
ext4_set_inode_flags // Set S_DAX flag
ext4_set_aops // Set aops to ext4_dax_aops
Existing file
-------------
lookup_open
ext4_lookup
ext4_iget
ext4_set_aops // Set aops to ext4_da_aops
ext4_set_inode_flags // Set S_DAX flag
So, we set ext4_da_aops for existing files since S_DAX flag is set after
ext4_set_aops().
Thanks,
-Toshi
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: open sets ext4_da_aops for DAX existing files
2018-09-07 21:23 open sets ext4_da_aops for DAX existing files Kani, Toshi
@ 2018-09-10 12:54 ` Jeff Moyer
2018-09-10 14:21 ` Kani, Toshi
2018-09-10 14:29 ` Jan Kara
1 sibling, 1 reply; 9+ messages in thread
From: Jeff Moyer @ 2018-09-10 12:54 UTC (permalink / raw)
To: Kani, Toshi; +Cc: linux-fsdevel, linux-nvdimm
"Kani, Toshi" <toshi.kani@hpe.com> writes:
> I noticed that both ext4_da_aops and ext4_dax_aops are used on DAX
> mounted ext4 files. Looking at open() path:
Eek. How did you notice this?
-Jeff
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: open sets ext4_da_aops for DAX existing files
2018-09-10 12:54 ` Jeff Moyer
@ 2018-09-10 14:21 ` Kani, Toshi
2018-09-10 14:26 ` Dan Williams
0 siblings, 1 reply; 9+ messages in thread
From: Kani, Toshi @ 2018-09-10 14:21 UTC (permalink / raw)
To: jmoyer; +Cc: linux-fsdevel, linux-nvdimm
On Mon, 2018-09-10 at 08:54 -0400, Jeff Moyer wrote:
> "Kani, Toshi" <toshi.kani@hpe.com> writes:
>
> > I noticed that both ext4_da_aops and ext4_dax_aops are used on DAX
> > mounted ext4 files. Looking at open() path:
>
> Eek. How did you notice this?
I tested sync path on DAX files to see if it flushes processor cache
properly. Then I noticed that it sometimes does, but sometimes doesn't.
Looking into function traces, I realized that ext4_dax_writepages() and
ext4_writepages() are called depending on their file status.
-Toshi
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: open sets ext4_da_aops for DAX existing files
2018-09-10 14:21 ` Kani, Toshi
@ 2018-09-10 14:26 ` Dan Williams
0 siblings, 0 replies; 9+ messages in thread
From: Dan Williams @ 2018-09-10 14:26 UTC (permalink / raw)
To: Kani, Toshi; +Cc: linux-fsdevel, linux-nvdimm
On Mon, Sep 10, 2018 at 7:21 AM, Kani, Toshi <toshi.kani@hpe.com> wrote:
> On Mon, 2018-09-10 at 08:54 -0400, Jeff Moyer wrote:
>> "Kani, Toshi" <toshi.kani@hpe.com> writes:
>>
>> > I noticed that both ext4_da_aops and ext4_dax_aops are used on DAX
>> > mounted ext4 files. Looking at open() path:
>>
>> Eek. How did you notice this?
>
> I tested sync path on DAX files to see if it flushes processor cache
> properly. Then I noticed that it sometimes does, but sometimes doesn't.
> Looking into function traces, I realized that ext4_dax_writepages() and
> ext4_writepages() are called depending on their file status.
Yikes, and nice find!
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: open sets ext4_da_aops for DAX existing files
2018-09-07 21:23 open sets ext4_da_aops for DAX existing files Kani, Toshi
2018-09-10 12:54 ` Jeff Moyer
@ 2018-09-10 14:29 ` Jan Kara
2018-09-10 14:51 ` Kani, Toshi
1 sibling, 1 reply; 9+ messages in thread
From: Jan Kara @ 2018-09-10 14:29 UTC (permalink / raw)
To: Kani, Toshi; +Cc: linux-fsdevel, linux-nvdimm
On Fri 07-09-18 21:23:19, Kani, Toshi wrote:
> I noticed that both ext4_da_aops and ext4_dax_aops are used on DAX
> mounted ext4 files. Looking at open() path:
>
> New file
> --------
> lookup_open
> ext4_create
> __ext4_new_inode
> ext4_set_inode_flags // Set S_DAX flag
> ext4_set_aops // Set aops to ext4_dax_aops
>
> Existing file
> -------------
> lookup_open
> ext4_lookup
> ext4_iget
> ext4_set_aops // Set aops to ext4_da_aops
> ext4_set_inode_flags // Set S_DAX flag
>
> So, we set ext4_da_aops for existing files since S_DAX flag is set after
> ext4_set_aops().
Good catch. Will you send a fix? I.e., call ext4_set_inode_flags() earlier
in the ext4_iget()? Did this bug have any user visible manifestations?
Please also add:
Fixes: 5f0663bb4a64f588f0a2dd6d1be68d40f9af0086
so that stable automation picks this up. Thanks!
Honza
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: open sets ext4_da_aops for DAX existing files
2018-09-10 14:29 ` Jan Kara
@ 2018-09-10 14:51 ` Kani, Toshi
2018-09-10 17:58 ` Elliott, Robert (Persistent Memory)
0 siblings, 1 reply; 9+ messages in thread
From: Kani, Toshi @ 2018-09-10 14:51 UTC (permalink / raw)
To: jack; +Cc: linux-fsdevel, linux-nvdimm
On Mon, 2018-09-10 at 16:29 +0200, Jan Kara wrote:
> On Fri 07-09-18 21:23:19, Kani, Toshi wrote:
> > I noticed that both ext4_da_aops and ext4_dax_aops are used on DAX
> > mounted ext4 files. Looking at open() path:
> >
> > New file
> > --------
> > lookup_open
> > ext4_create
> > __ext4_new_inode
> > ext4_set_inode_flags // Set S_DAX flag
> > ext4_set_aops // Set aops to ext4_dax_aops
> >
> > Existing file
> > -------------
> > lookup_open
> > ext4_lookup
> > ext4_iget
> > ext4_set_aops // Set aops to ext4_da_aops
> > ext4_set_inode_flags // Set S_DAX flag
> >
> > So, we set ext4_da_aops for existing files since S_DAX flag is set after
> > ext4_set_aops().
>
> Good catch. Will you send a fix? I.e., call ext4_set_inode_flags() earlier
> in the ext4_iget()? Did this bug have any user visible manifestations?
Yes, sync did not flush processor cache.
> Please also add:
>
> Fixes: 5f0663bb4a64f588f0a2dd6d1be68d40f9af0086
>
> so that stable automation picks this up. Thanks!
>
Will do.
Thanks,
-Toshi
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: open sets ext4_da_aops for DAX existing files
2018-09-10 14:51 ` Kani, Toshi
@ 2018-09-10 17:58 ` Elliott, Robert (Persistent Memory)
2018-09-11 15:27 ` Jan Kara
0 siblings, 1 reply; 9+ messages in thread
From: Elliott, Robert (Persistent Memory) @ 2018-09-10 17:58 UTC (permalink / raw)
To: Kani, Toshi, jack; +Cc: linux-fsdevel, linux-nvdimm
> -----Original Message-----
> From: Linux-nvdimm [mailto:linux-nvdimm-bounces@lists.01.org] On Behalf Of Kani, Toshi
> Sent: Monday, September 10, 2018 9:52 AM
> To: jack@suse.cz
> Cc: linux-fsdevel@vger.kernel.org; linux-nvdimm@lists.01.org
> Subject: Re: open sets ext4_da_aops for DAX existing files
...
> > Good catch. Will you send a fix? I.e., call ext4_set_inode_flags() earlier
> > in the ext4_iget()? Did this bug have any user visible manifestations?
>
> Yes, sync did not flush processor cache.
>
> > Please also add:
> >
> > Fixes: 5f0663bb4a64f588f0a2dd6d1be68d40f9af0086
> >
> > so that stable automation picks this up. Thanks!
> >
>
> Will do.
>
Also check if the same problem exists with ext2.
---
Robert Elliott, HPE Persistent Memory
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: open sets ext4_da_aops for DAX existing files
2018-09-10 17:58 ` Elliott, Robert (Persistent Memory)
@ 2018-09-11 15:27 ` Jan Kara
2018-09-11 16:34 ` Kani, Toshi
0 siblings, 1 reply; 9+ messages in thread
From: Jan Kara @ 2018-09-11 15:27 UTC (permalink / raw)
To: Elliott, Robert (Persistent Memory); +Cc: linux-fsdevel, jack, linux-nvdimm
On Mon 10-09-18 17:58:10, Elliott, Robert (Persistent Memory) wrote:
>
>
> > -----Original Message-----
> > From: Linux-nvdimm [mailto:linux-nvdimm-bounces@lists.01.org] On Behalf Of Kani, Toshi
> > Sent: Monday, September 10, 2018 9:52 AM
> > To: jack@suse.cz
> > Cc: linux-fsdevel@vger.kernel.org; linux-nvdimm@lists.01.org
> > Subject: Re: open sets ext4_da_aops for DAX existing files
> ...
> > > Good catch. Will you send a fix? I.e., call ext4_set_inode_flags() earlier
> > > in the ext4_iget()? Did this bug have any user visible manifestations?
> >
> > Yes, sync did not flush processor cache.
> >
> > > Please also add:
> > >
> > > Fixes: 5f0663bb4a64f588f0a2dd6d1be68d40f9af0086
> > >
> > > so that stable automation picks this up. Thanks!
> > >
> >
> > Will do.
> >
>
> Also check if the same problem exists with ext2.
The problem also exists in ext2 so that needs fixing as well.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: open sets ext4_da_aops for DAX existing files
2018-09-11 15:27 ` Jan Kara
@ 2018-09-11 16:34 ` Kani, Toshi
0 siblings, 0 replies; 9+ messages in thread
From: Kani, Toshi @ 2018-09-11 16:34 UTC (permalink / raw)
To: jack, Elliott, Robert (Persistent Memory); +Cc: linux-fsdevel, linux-nvdimm
On Tue, 2018-09-11 at 17:27 +0200, Jan Kara wrote:
> On Mon 10-09-18 17:58:10, Elliott, Robert (Persistent Memory) wrote:
> >
> >
> > > -----Original Message-----
> > > From: Linux-nvdimm [mailto:linux-nvdimm-bounces@lists.01.org] On Behalf Of Kani, Toshi
> > > Sent: Monday, September 10, 2018 9:52 AM
> > > To: jack@suse.cz
> > > Cc: linux-fsdevel@vger.kernel.org; linux-nvdimm@lists.01.org
> > > Subject: Re: open sets ext4_da_aops for DAX existing files
> >
> > ...
> > > > Good catch. Will you send a fix? I.e., call ext4_set_inode_flags() earlier
> > > > in the ext4_iget()? Did this bug have any user visible manifestations?
> > >
> > > Yes, sync did not flush processor cache.
> > >
> > > > Please also add:
> > > >
> > > > Fixes: 5f0663bb4a64f588f0a2dd6d1be68d40f9af0086
> > > >
> > > > so that stable automation picks this up. Thanks!
> > > >
> > >
> > > Will do.
> > >
> >
> > Also check if the same problem exists with ext2.
>
> The problem also exists in ext2 so that needs fixing as well.
Right. Once my ext4 patch 2/2 is reviewed, I will duplicate it for
ext2.
Thanks,
-Toshi
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-09-11 16:34 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-07 21:23 open sets ext4_da_aops for DAX existing files Kani, Toshi
2018-09-10 12:54 ` Jeff Moyer
2018-09-10 14:21 ` Kani, Toshi
2018-09-10 14:26 ` Dan Williams
2018-09-10 14:29 ` Jan Kara
2018-09-10 14:51 ` Kani, Toshi
2018-09-10 17:58 ` Elliott, Robert (Persistent Memory)
2018-09-11 15:27 ` Jan Kara
2018-09-11 16:34 ` Kani, Toshi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).