* Bug in page_cache_tree_delete?
@ 2016-11-08 10:35 Jan Kara
2016-11-08 13:03 ` Kirill A. Shutemov
0 siblings, 1 reply; 3+ messages in thread
From: Jan Kara @ 2016-11-08 10:35 UTC (permalink / raw)
To: Kirill A. Shutemov; +Cc: linux-mm, Andrew Morton
Hi Kirill,
I've noticed that your commit 83929372f629 (filemap: prepare find and
delete operations for huge pages) added to page_cache_tree_delete():
int i, nr = PageHuge(page) ? 1 : hpage_nr_pages(page);
Isn't the logic computing 'nr' inverted? I'd expect that if page is
!PageHuge, we want to delete just one page... OTOH I'm surprised this would
not blow up anywhere if it was really wrong so maybe I just miss something.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Bug in page_cache_tree_delete?
2016-11-08 10:35 Bug in page_cache_tree_delete? Jan Kara
@ 2016-11-08 13:03 ` Kirill A. Shutemov
2016-11-08 13:51 ` Jan Kara
0 siblings, 1 reply; 3+ messages in thread
From: Kirill A. Shutemov @ 2016-11-08 13:03 UTC (permalink / raw)
To: Jan Kara; +Cc: Kirill A. Shutemov, linux-mm, Andrew Morton
On Tue, Nov 08, 2016 at 11:35:41AM +0100, Jan Kara wrote:
> Hi Kirill,
>
> I've noticed that your commit 83929372f629 (filemap: prepare find and
> delete operations for huge pages) added to page_cache_tree_delete():
>
> int i, nr = PageHuge(page) ? 1 : hpage_nr_pages(page);
>
> Isn't the logic computing 'nr' inverted? I'd expect that if page is
> !PageHuge, we want to delete just one page... OTOH I'm surprised this would
> not blow up anywhere if it was really wrong so maybe I just miss something.
No, that's not bug.
PageHuge() detects hugetlb pages (we probably should rename the helper)
which represented by one entry on radix-tree per huge page.
--
Kirill A. Shutemov
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Bug in page_cache_tree_delete?
2016-11-08 13:03 ` Kirill A. Shutemov
@ 2016-11-08 13:51 ` Jan Kara
0 siblings, 0 replies; 3+ messages in thread
From: Jan Kara @ 2016-11-08 13:51 UTC (permalink / raw)
To: Kirill A. Shutemov; +Cc: Jan Kara, Kirill A. Shutemov, linux-mm, Andrew Morton
On Tue 08-11-16 16:03:07, Kirill A. Shutemov wrote:
> On Tue, Nov 08, 2016 at 11:35:41AM +0100, Jan Kara wrote:
> > Hi Kirill,
> >
> > I've noticed that your commit 83929372f629 (filemap: prepare find and
> > delete operations for huge pages) added to page_cache_tree_delete():
> >
> > int i, nr = PageHuge(page) ? 1 : hpage_nr_pages(page);
> >
> > Isn't the logic computing 'nr' inverted? I'd expect that if page is
> > !PageHuge, we want to delete just one page... OTOH I'm surprised this would
> > not blow up anywhere if it was really wrong so maybe I just miss something.
>
> No, that's not bug.
>
> PageHuge() detects hugetlb pages (we probably should rename the helper)
> which represented by one entry on radix-tree per huge page.
Ah, right. Thanks for explanation! Maybe that line would deserve a comment?
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-11-08 13:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-08 10:35 Bug in page_cache_tree_delete? Jan Kara
2016-11-08 13:03 ` Kirill A. Shutemov
2016-11-08 13:51 ` Jan Kara
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.