On Tue, Jan 19, 2021 at 06:54:28AM +0800, Qu Wenruo wrote: > On 2021/1/19 上午6:46, David Sterba wrote: > > On Sat, Jan 16, 2021 at 03:15:18PM +0800, Qu Wenruo wrote: > >> + return; > >> + > >> + subpage = (struct btrfs_subpage *)detach_page_private(page); > >> + ASSERT(subpage); > >> + kfree(subpage); > >> +} > >> diff --git a/fs/btrfs/subpage.h b/fs/btrfs/subpage.h > >> new file mode 100644 > >> index 000000000000..96f3b226913e > >> --- /dev/null > >> +++ b/fs/btrfs/subpage.h > >> @@ -0,0 +1,31 @@ > >> +/* SPDX-License-Identifier: GPL-2.0 */ > >> + > >> +#ifndef BTRFS_SUBPAGE_H > >> +#define BTRFS_SUBPAGE_H > >> + > >> +#include <linux/spinlock.h> > >> +#include "ctree.h" > > > > So subpage.h would pull the whole ctree.h, that's not very nice. If > > anything, the .c could include ctree.h because there are lots of the > > common structure and function definitions, but not the .h. This creates > > unnecessary include dependencies. > > > > Any pointer type you'd need in structures could be forward declared. > > Unfortunately, the main needed pointer is fs_info, and we're accessing > it pretty frequently (mostly for sector/node size). > > I don't believe forward declaration would help in this case.
I've looked at the final subpage.h and you add way too many static inlines that don't seem to be necessary for the reasons the static inlines are supposed to be used.
