* Oddities in do_read_cache_page()
@ 2022-06-27 9:12 Hannes Reinecke
[not found] ` <CAFhKne_hYFU0g5_68R=FA_QRWQc8ZRM8eCTNFwhP+4p4YHhZ8Q@mail.gmail.com>
0 siblings, 1 reply; 2+ messages in thread
From: Hannes Reinecke @ 2022-06-27 9:12 UTC (permalink / raw
To: Matthew Wilcox, Mel Gorman, Linux Kernel, Jan Kara
Hey Matt,
I've stumbled across this code in do_read_cache_page():
struct folio *folio;
folio = do_read_cache_folio(mapping, index, filler, file, gfp);
if (IS_ERR(folio))
return &folio->page;
return folio_file_page(folio, index);
Following 'do_read_cache_folio()' I see that it does things like
folio = filemap_alloc_folio(gfp, 0);
if (!folio)
return ERR_PTR(-ENOMEM);
Now I freely admit that my knowledge of folios is hazy at best, but
dereferencing an error pointer is something I would seriously frown upon
if I were to review the code.
Care to explain?
Or is it, indeed, simply a bug?
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: Oddities in do_read_cache_page()
[not found] ` <CAFhKne_hYFU0g5_68R=FA_QRWQc8ZRM8eCTNFwhP+4p4YHhZ8Q@mail.gmail.com>
@ 2022-06-27 12:55 ` Jan Kara
0 siblings, 0 replies; 2+ messages in thread
From: Jan Kara @ 2022-06-27 12:55 UTC (permalink / raw
To: Matthew Wilcox
Cc: Hannes Reinecke, Matthew Wilcox, Mel Gorman, Linux Kernel,
Jan Kara
Hi Matthew!
On Mon 27-06-22 07:17:35, Matthew Wilcox wrote:
> &folio->page does not dereference the pointer, it simply performs
> arithmetic on it. In this case, adding zero. It also changes the type from
> folio to page. There's no bug here.
I agree there's no functional bug there (at least yet). But arguably this
would be much more readable and definitely more future-proof as:
return (struct page *)ERR_CAST(folio);
Because even doing arithmetics on error pointer is fishy...
Honza
>
> On Mon., Jun. 27, 2022, 04:12 Hannes Reinecke, <hare@suse.de> wrote:
>
> > Hey Matt,
> >
> > I've stumbled across this code in do_read_cache_page():
> >
> > struct folio *folio;
> >
> > folio = do_read_cache_folio(mapping, index, filler, file, gfp);
> > if (IS_ERR(folio))
> > return &folio->page;
> > return folio_file_page(folio, index);
> >
> > Following 'do_read_cache_folio()' I see that it does things like
> >
> > folio = filemap_alloc_folio(gfp, 0);
> > if (!folio)
> > return ERR_PTR(-ENOMEM);
> >
> > Now I freely admit that my knowledge of folios is hazy at best, but
> > dereferencing an error pointer is something I would seriously frown upon
> > if I were to review the code.
> > Care to explain?
> > Or is it, indeed, simply a bug?
> >
> > Cheers,
> >
> > Hannes
> > --
> > Dr. Hannes Reinecke Kernel Storage Architect
> > hare@suse.de +49 911 74053 688
> > SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
> > HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
> >
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-06-27 12:55 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-06-27 9:12 Oddities in do_read_cache_page() Hannes Reinecke
[not found] ` <CAFhKne_hYFU0g5_68R=FA_QRWQc8ZRM8eCTNFwhP+4p4YHhZ8Q@mail.gmail.com>
2022-06-27 12:55 ` Jan Kara
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 read-only IMAP folder(s) and NNTP newsgroup(s).