Messages in this thread | | | Date | Mon, 27 Jun 2022 14:55:33 +0200 | From | Jan Kara <> | Subject | Re: Oddities in do_read_cache_page() |
| |
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
| |