diff mbox series

mm: remove redundant if overhead

Message ID 20240910143945.1123-1-justinjiang@vivo.com (mailing list archive)
State New
Headers show
Series mm: remove redundant if overhead | expand

Commit Message

zhiguojiang Sept. 10, 2024, 2:39 p.m. UTC
Remove redundant if judgment overhead.

Signed-off-by: Zhiguo Jiang <justinjiang@vivo.com>
---
 mm/page-writeback.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Matthew Wilcox (Oracle) Sept. 10, 2024, 4:03 p.m. UTC | #1
On Tue, Sep 10, 2024 at 10:39:45PM +0800, Zhiguo Jiang wrote:
> Remove redundant if judgment overhead.

It's not redundant; it avoids dirtying the cacheline if the folio
is already marked as dirty.

>  bool noop_dirty_folio(struct address_space *mapping, struct folio *folio)
>  {
> -	if (!folio_test_dirty(folio))
> -		return !folio_test_set_dirty(folio);
> -	return false;
> +	return !folio_test_set_dirty(folio);
>  }
zhiguojiang Sept. 11, 2024, 1:14 a.m. UTC | #2
在 2024/9/11 0:03, Matthew Wilcox 写道:
> On Tue, Sep 10, 2024 at 10:39:45PM +0800, Zhiguo Jiang wrote:
>> Remove redundant if judgment overhead.
> It's not redundant; it avoids dirtying the cacheline if the folio
> is already marked as dirty.
Ok, Is it necessary to add comments here to explain and avoid readers'
misunderstandings? E.g. 'Avoid dirtying the cacheline if the folio is
already marked as dirty.'
>>   bool noop_dirty_folio(struct address_space *mapping, struct folio *folio)
>>   {
>> -	if (!folio_test_dirty(folio))
>> -		return !folio_test_set_dirty(folio);
>> -	return false;
>> +	return !folio_test_set_dirty(folio);
>>   }
Thanks
Zhiguo
Matthew Wilcox (Oracle) Sept. 11, 2024, 1:34 a.m. UTC | #3
On Wed, Sep 11, 2024 at 09:14:10AM +0800, zhiguojiang wrote:
> 在 2024/9/11 0:03, Matthew Wilcox 写道:
> > On Tue, Sep 10, 2024 at 10:39:45PM +0800, Zhiguo Jiang wrote:
> > > Remove redundant if judgment overhead.
> > It's not redundant; it avoids dirtying the cacheline if the folio
> > is already marked as dirty.

> Ok, Is it necessary to add comments here to explain and avoid readers'
> misunderstandings? E.g. 'Avoid dirtying the cacheline if the folio is
> already marked as dirty.'

No, it's a fairly common pattern to test-and-test-and-set(or clear)
diff mbox series

Patch

diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index fcd4c1439cb9..2d2c3f4e640d
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2715,9 +2715,7 @@  int do_writepages(struct address_space *mapping, struct writeback_control *wbc)
  */
 bool noop_dirty_folio(struct address_space *mapping, struct folio *folio)
 {
-	if (!folio_test_dirty(folio))
-		return !folio_test_set_dirty(folio);
-	return false;
+	return !folio_test_set_dirty(folio);
 }
 EXPORT_SYMBOL(noop_dirty_folio);