* [f2fs-dev] [PATCH v2 3/4] f2fs: atomic: fix to truncate pagecache before on-disk metadata truncation
@ 2024-07-26 1:22 Chao Yu
2024-07-29 16:26 ` Jaegeuk Kim
0 siblings, 1 reply; 6+ messages in thread
From: Chao Yu @ 2024-07-26 1:22 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel
We should always truncate pagecache while truncating on-disk data.
Fixes: a46bebd502fe ("f2fs: synchronize atomic write aborts")
Signed-off-by: Chao Yu <chao@kernel.org>
---
v2:
- fix to use cow_inode instead of inode
fs/f2fs/file.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 6c62f76474d1..54886ddcb8ab 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2202,6 +2202,7 @@ static int f2fs_ioc_start_atomic_write(struct file *filp, bool truncate)
F2FS_I(fi->cow_inode)->atomic_inode = inode;
} else {
/* Reuse the already created COW inode */
+ truncate_setsize(fi->cow_inode, 0);
ret = f2fs_do_truncate_blocks(fi->cow_inode, 0, true);
if (ret) {
f2fs_up_write(&fi->i_gc_rwsem[WRITE]);
--
2.40.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [f2fs-dev] [PATCH v2 3/4] f2fs: atomic: fix to truncate pagecache before on-disk metadata truncation
2024-07-26 1:22 [f2fs-dev] [PATCH v2 3/4] f2fs: atomic: fix to truncate pagecache before on-disk metadata truncation Chao Yu
@ 2024-07-29 16:26 ` Jaegeuk Kim
2024-07-30 1:29 ` Chao Yu
0 siblings, 1 reply; 6+ messages in thread
From: Jaegeuk Kim @ 2024-07-29 16:26 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
On 07/26, Chao Yu wrote:
> We should always truncate pagecache while truncating on-disk data.
>
> Fixes: a46bebd502fe ("f2fs: synchronize atomic write aborts")
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
> v2:
> - fix to use cow_inode instead of inode
> fs/f2fs/file.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 6c62f76474d1..54886ddcb8ab 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -2202,6 +2202,7 @@ static int f2fs_ioc_start_atomic_write(struct file *filp, bool truncate)
> F2FS_I(fi->cow_inode)->atomic_inode = inode;
> } else {
> /* Reuse the already created COW inode */
> + truncate_setsize(fi->cow_inode, 0);
What if the below truncation failed?
> ret = f2fs_do_truncate_blocks(fi->cow_inode, 0, true);
> if (ret) {
> f2fs_up_write(&fi->i_gc_rwsem[WRITE]);
> --
> 2.40.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [f2fs-dev] [PATCH v2 3/4] f2fs: atomic: fix to truncate pagecache before on-disk metadata truncation
2024-07-29 16:26 ` Jaegeuk Kim
@ 2024-07-30 1:29 ` Chao Yu
2024-07-30 16:26 ` Jaegeuk Kim
0 siblings, 1 reply; 6+ messages in thread
From: Chao Yu @ 2024-07-30 1:29 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel
On 2024/7/30 0:26, Jaegeuk Kim wrote:
> On 07/26, Chao Yu wrote:
>> We should always truncate pagecache while truncating on-disk data.
>>
>> Fixes: a46bebd502fe ("f2fs: synchronize atomic write aborts")
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>> v2:
>> - fix to use cow_inode instead of inode
>> fs/f2fs/file.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index 6c62f76474d1..54886ddcb8ab 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -2202,6 +2202,7 @@ static int f2fs_ioc_start_atomic_write(struct file *filp, bool truncate)
>> F2FS_I(fi->cow_inode)->atomic_inode = inode;
>> } else {
>> /* Reuse the already created COW inode */
>> + truncate_setsize(fi->cow_inode, 0);
>
> What if the below truncation failed?
What about just dropping page cache and do not set isize to 0?
Thanks,
>
>> ret = f2fs_do_truncate_blocks(fi->cow_inode, 0, true);
>> if (ret) {
>> f2fs_up_write(&fi->i_gc_rwsem[WRITE]);
>> --
>> 2.40.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [f2fs-dev] [PATCH v2 3/4] f2fs: atomic: fix to truncate pagecache before on-disk metadata truncation
2024-07-30 1:29 ` Chao Yu
@ 2024-07-30 16:26 ` Jaegeuk Kim
2024-07-31 8:18 ` Chao Yu
0 siblings, 1 reply; 6+ messages in thread
From: Jaegeuk Kim @ 2024-07-30 16:26 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
On 07/30, Chao Yu wrote:
> On 2024/7/30 0:26, Jaegeuk Kim wrote:
> > On 07/26, Chao Yu wrote:
> > > We should always truncate pagecache while truncating on-disk data.
> > >
> > > Fixes: a46bebd502fe ("f2fs: synchronize atomic write aborts")
> > > Signed-off-by: Chao Yu <chao@kernel.org>
> > > ---
> > > v2:
> > > - fix to use cow_inode instead of inode
> > > fs/f2fs/file.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > > index 6c62f76474d1..54886ddcb8ab 100644
> > > --- a/fs/f2fs/file.c
> > > +++ b/fs/f2fs/file.c
> > > @@ -2202,6 +2202,7 @@ static int f2fs_ioc_start_atomic_write(struct file *filp, bool truncate)
> > > F2FS_I(fi->cow_inode)->atomic_inode = inode;
> > > } else {
> > > /* Reuse the already created COW inode */
> > > + truncate_setsize(fi->cow_inode, 0);
> >
> > What if the below truncation failed?
>
> What about just dropping page cache and do not set isize to 0?
Can we also check if there's any dirty page before truncating it?
>
> Thanks,
>
> >
> > > ret = f2fs_do_truncate_blocks(fi->cow_inode, 0, true);
> > > if (ret) {
> > > f2fs_up_write(&fi->i_gc_rwsem[WRITE]);
> > > --
> > > 2.40.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [f2fs-dev] [PATCH v2 3/4] f2fs: atomic: fix to truncate pagecache before on-disk metadata truncation
2024-07-30 16:26 ` Jaegeuk Kim
@ 2024-07-31 8:18 ` Chao Yu
2024-07-31 20:27 ` Jaegeuk Kim
0 siblings, 1 reply; 6+ messages in thread
From: Chao Yu @ 2024-07-31 8:18 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel
On 2024/7/31 0:26, Jaegeuk Kim wrote:
> On 07/30, Chao Yu wrote:
>> On 2024/7/30 0:26, Jaegeuk Kim wrote:
>>> On 07/26, Chao Yu wrote:
>>>> We should always truncate pagecache while truncating on-disk data.
>>>>
>>>> Fixes: a46bebd502fe ("f2fs: synchronize atomic write aborts")
>>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>>> ---
>>>> v2:
>>>> - fix to use cow_inode instead of inode
>>>> fs/f2fs/file.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>> index 6c62f76474d1..54886ddcb8ab 100644
>>>> --- a/fs/f2fs/file.c
>>>> +++ b/fs/f2fs/file.c
>>>> @@ -2202,6 +2202,7 @@ static int f2fs_ioc_start_atomic_write(struct file *filp, bool truncate)
>>>> F2FS_I(fi->cow_inode)->atomic_inode = inode;
>>>> } else {
>>>> /* Reuse the already created COW inode */
>>>> + truncate_setsize(fi->cow_inode, 0);
>>>
>>> What if the below truncation failed?
>>
>> What about just dropping page cache and do not set isize to 0?
>
> Can we also check if there's any dirty page before truncating it?
Sure, so how about this?
/* Reuse the already created COW inode */
if (get_dirty_pages(fi->cow_inode)) {
ret = filemap_write_and_wait(fi->cow_inode->i_mapping);
if (ret) {
f2fs_up_write(&fi->i_gc_rwsem[WRITE]);
goto out;
}
}
truncate_inode_pages(fi->cow_inode->i_mapping, 0);
ret = f2fs_do_truncate_blocks(fi->cow_inode, 0, true);
if (ret) {
f2fs_up_write(&fi->i_gc_rwsem[WRITE]);
goto out;
}
i_size_write(fi->cow_inode, 0);
Thanks,
>
>>
>> Thanks,
>>
>>>
>>>> ret = f2fs_do_truncate_blocks(fi->cow_inode, 0, true);
>>>> if (ret) {
>>>> f2fs_up_write(&fi->i_gc_rwsem[WRITE]);
>>>> --
>>>> 2.40.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [f2fs-dev] [PATCH v2 3/4] f2fs: atomic: fix to truncate pagecache before on-disk metadata truncation
2024-07-31 8:18 ` Chao Yu
@ 2024-07-31 20:27 ` Jaegeuk Kim
0 siblings, 0 replies; 6+ messages in thread
From: Jaegeuk Kim @ 2024-07-31 20:27 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
On 07/31, Chao Yu wrote:
> On 2024/7/31 0:26, Jaegeuk Kim wrote:
> > On 07/30, Chao Yu wrote:
> > > On 2024/7/30 0:26, Jaegeuk Kim wrote:
> > > > On 07/26, Chao Yu wrote:
> > > > > We should always truncate pagecache while truncating on-disk data.
> > > > >
> > > > > Fixes: a46bebd502fe ("f2fs: synchronize atomic write aborts")
> > > > > Signed-off-by: Chao Yu <chao@kernel.org>
> > > > > ---
> > > > > v2:
> > > > > - fix to use cow_inode instead of inode
> > > > > fs/f2fs/file.c | 1 +
> > > > > 1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > > > > index 6c62f76474d1..54886ddcb8ab 100644
> > > > > --- a/fs/f2fs/file.c
> > > > > +++ b/fs/f2fs/file.c
> > > > > @@ -2202,6 +2202,7 @@ static int f2fs_ioc_start_atomic_write(struct file *filp, bool truncate)
> > > > > F2FS_I(fi->cow_inode)->atomic_inode = inode;
> > > > > } else {
> > > > > /* Reuse the already created COW inode */
> > > > > + truncate_setsize(fi->cow_inode, 0);
> > > >
> > > > What if the below truncation failed?
> > >
> > > What about just dropping page cache and do not set isize to 0?
> >
> > Can we also check if there's any dirty page before truncating it?
>
> Sure, so how about this?
>
> /* Reuse the already created COW inode */
> if (get_dirty_pages(fi->cow_inode)) {
Was supposed to be a bug here?
> ret = filemap_write_and_wait(fi->cow_inode->i_mapping);
> if (ret) {
> f2fs_up_write(&fi->i_gc_rwsem[WRITE]);
> goto out;
> }
> }
>
> truncate_inode_pages(fi->cow_inode->i_mapping, 0);
invalidate_mapping_pages() would be much safer?
>
> ret = f2fs_do_truncate_blocks(fi->cow_inode, 0, true);
> if (ret) {
> f2fs_up_write(&fi->i_gc_rwsem[WRITE]);
> goto out;
> }
>
> i_size_write(fi->cow_inode, 0);
>
> Thanks,
>
> >
> > >
> > > Thanks,
> > >
> > > >
> > > > > ret = f2fs_do_truncate_blocks(fi->cow_inode, 0, true);
> > > > > if (ret) {
> > > > > f2fs_up_write(&fi->i_gc_rwsem[WRITE]);
> > > > > --
> > > > > 2.40.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-07-31 20:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-26 1:22 [f2fs-dev] [PATCH v2 3/4] f2fs: atomic: fix to truncate pagecache before on-disk metadata truncation Chao Yu
2024-07-29 16:26 ` Jaegeuk Kim
2024-07-30 1:29 ` Chao Yu
2024-07-30 16:26 ` Jaegeuk Kim
2024-07-31 8:18 ` Chao Yu
2024-07-31 20:27 ` Jaegeuk Kim
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).