On Mon, Dec 12, 2022 at 8:24 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > On Mon, Dec 12, 2022 at 08:23:31AM -0800, Nhat Pham wrote: > > > It would be easier to read if you inverted the condition here. > > > > Oh I think I tried > > > > if (!f.file) > > return -EBADF; > > > > here, but there are some mixing-code-with-decl warnings. > > If I recall correctly, the problem is with this line: > > > > XA_STATE(xas, &mapping->i_pages, first_index); > > > > which is expanded into a declaration: > > > > #define XA_STATE(name, array, index) \ > > struct xa_state name = __XA_STATE(array, index, 0, 0) > > > > It requires a valid mapping though, which is > > obtained from f.file: > > > > struct address_space *mapping = f.file->f_mapping; > > > > so it cannot be moved above the if(!f.file) check either... > > Perhaps you're trying to do too much in a single function? That's fair. In an ancient version of this, I did try to separate the folio-walking logic into its own function, but then decided against it because it seemed unnecessary. That was before the complexity of that piece of logic blew up to the current situation though, so perhaps it is time to revisit this decision and refactor it.
