| Message ID | 20231119194740.94101-14-ryncsn@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Swapin path refactor for optimization and bugfix | expand |
Hi Kairui, I agree the resulting code is marginally better. However, this change does not bring enough value to justify a stand alone patch. It has no impact on the resulting kernel. It would be much better if the code was checkin like this, or if you are modifying this function, rewrite it better. In my opinion, doing very trivial code shuffling for the sake of cleaning up is not justifiable for the value it brings. For one it will make the git blame less obvious who actually changed that code for what reason. I am against trivial code shuffling. Chris On Sun, Nov 19, 2023 at 11:48 AM Kairui Song <ryncsn@gmail.com> wrote: > > From: Kairui Song <kasong@tencent.com> > > Rearrange the if statement, reduce the code indent, no feature change. > > Signed-off-by: Kairui Song <kasong@tencent.com> > --- > mm/swap_state.c | 58 ++++++++++++++++++++++++------------------------- > 1 file changed, 28 insertions(+), 30 deletions(-) > > diff --git a/mm/swap_state.c b/mm/swap_state.c > index 91461e26a8cc..3b5a34f47192 100644 > --- a/mm/swap_state.c > +++ b/mm/swap_state.c > @@ -336,41 +336,39 @@ static inline bool swap_use_vma_readahead(struct swap_info_struct *si) > */ > struct folio *swap_cache_get_folio(swp_entry_t entry, struct vm_fault *vmf) > { > + bool vma_ra, readahead; > struct folio *folio; > > folio = filemap_get_folio(swap_address_space(entry), swp_offset(entry)); > - if (!IS_ERR(folio)) { > - bool vma_ra = swap_use_vma_readahead(swp_swap_info(entry)); > - bool readahead; > + if (IS_ERR(folio)) > + return NULL; > > - /* > - * At the moment, we don't support PG_readahead for anon THP > - * so let's bail out rather than confusing the readahead stat. > - */ > - if (unlikely(folio_test_large(folio))) > - return folio; > - > - readahead = folio_test_clear_readahead(folio); > - if (vmf && vma_ra) { > - unsigned long ra_val; > - int win, hits; > - > - ra_val = GET_SWAP_RA_VAL(vmf->vma); > - win = SWAP_RA_WIN(ra_val); > - hits = SWAP_RA_HITS(ra_val); > - if (readahead) > - hits = min_t(int, hits + 1, SWAP_RA_HITS_MAX); > - atomic_long_set(&vmf->vma->swap_readahead_info, > - SWAP_RA_VAL(vmf->address, win, hits)); > - } > + /* > + * At the moment, we don't support PG_readahead for anon THP > + * so let's bail out rather than confusing the readahead stat. > + */ > + if (unlikely(folio_test_large(folio))) > + return folio; > > - if (readahead) { > - count_vm_event(SWAP_RA_HIT); > - if (!vmf || !vma_ra) > - atomic_inc(&swapin_readahead_hits); > - } > - } else { > - folio = NULL; > + vma_ra = swap_use_vma_readahead(swp_swap_info(entry)); > + readahead = folio_test_clear_readahead(folio); > + if (vmf && vma_ra) { > + unsigned long ra_val; > + int win, hits; > + > + ra_val = GET_SWAP_RA_VAL(vmf->vma); > + win = SWAP_RA_WIN(ra_val); > + hits = SWAP_RA_HITS(ra_val); > + if (readahead) > + hits = min_t(int, hits + 1, SWAP_RA_HITS_MAX); > + atomic_long_set(&vmf->vma->swap_readahead_info, > + SWAP_RA_VAL(vmf->address, win, hits)); > + } > + > + if (readahead) { > + count_vm_event(SWAP_RA_HIT); > + if (!vmf || !vma_ra) > + atomic_inc(&swapin_readahead_hits); > } > > return folio; > -- > 2.42.0 > >
diff --git a/mm/swap_state.c b/mm/swap_state.c index 91461e26a8cc..3b5a34f47192 100644 --- a/mm/swap_state.c +++ b/mm/swap_state.c @@ -336,41 +336,39 @@ static inline bool swap_use_vma_readahead(struct swap_info_struct *si) */ struct folio *swap_cache_get_folio(swp_entry_t entry, struct vm_fault *vmf) { + bool vma_ra, readahead; struct folio *folio; folio = filemap_get_folio(swap_address_space(entry), swp_offset(entry)); - if (!IS_ERR(folio)) { - bool vma_ra = swap_use_vma_readahead(swp_swap_info(entry)); - bool readahead; + if (IS_ERR(folio)) + return NULL; - /* - * At the moment, we don't support PG_readahead for anon THP - * so let's bail out rather than confusing the readahead stat. - */ - if (unlikely(folio_test_large(folio))) - return folio; - - readahead = folio_test_clear_readahead(folio); - if (vmf && vma_ra) { - unsigned long ra_val; - int win, hits; - - ra_val = GET_SWAP_RA_VAL(vmf->vma); - win = SWAP_RA_WIN(ra_val); - hits = SWAP_RA_HITS(ra_val); - if (readahead) - hits = min_t(int, hits + 1, SWAP_RA_HITS_MAX); - atomic_long_set(&vmf->vma->swap_readahead_info, - SWAP_RA_VAL(vmf->address, win, hits)); - } + /* + * At the moment, we don't support PG_readahead for anon THP + * so let's bail out rather than confusing the readahead stat. + */ + if (unlikely(folio_test_large(folio))) + return folio; - if (readahead) { - count_vm_event(SWAP_RA_HIT); - if (!vmf || !vma_ra) - atomic_inc(&swapin_readahead_hits); - } - } else { - folio = NULL; + vma_ra = swap_use_vma_readahead(swp_swap_info(entry)); + readahead = folio_test_clear_readahead(folio); + if (vmf && vma_ra) { + unsigned long ra_val; + int win, hits; + + ra_val = GET_SWAP_RA_VAL(vmf->vma); + win = SWAP_RA_WIN(ra_val); + hits = SWAP_RA_HITS(ra_val); + if (readahead) + hits = min_t(int, hits + 1, SWAP_RA_HITS_MAX); + atomic_long_set(&vmf->vma->swap_readahead_info, + SWAP_RA_VAL(vmf->address, win, hits)); + } + + if (readahead) { + count_vm_event(SWAP_RA_HIT); + if (!vmf || !vma_ra) + atomic_inc(&swapin_readahead_hits); } return folio;