Re: [PATCH v4 11/18] fs, dax: introduce DEFINE_FSDAX_AOPS

[Dan Williams <dan.j.williams@intel.com>]

On Tue, Dec 26, 2017 at 9:29 PM, Matthew Wilcox <willy@infradead.org> wrote:
> On Sat, Dec 23, 2017 at 04:56:59PM -0800, Dan Williams wrote:
>> +int dax_set_page_dirty(struct page *page)
>> +{
>> +     /*
>> +      * Unlike __set_page_dirty_no_writeback, dax does all dirty
>> +      * tracking in the radix in response to mkwrite faults.
>
> Please stop saying "in the radix".  I think you mean "in the page cache".

Ok, I'll be more precise and mention the PAGECACHE_TAG_DIRTY vs
PageDirty distinction.

>
>> +EXPORT_SYMBOL(dax_set_page_dirty);
>> +EXPORT_SYMBOL(dax_direct_IO);
>> +EXPORT_SYMBOL(dax_writepage);
>> +EXPORT_SYMBOL(dax_readpage);
>> +EXPORT_SYMBOL(dax_readpages);
>> +EXPORT_SYMBOL(dax_write_begin);
>> +EXPORT_SYMBOL(dax_write_end);
>> +EXPORT_SYMBOL(dax_invalidatepage);
>
> Exporting all these symbols to modules isn't exactly free.  Are you sure it
> doesn't make more sense to put tests for dax in the existing aops?
>

I'd rather have just one global fs_dax_aops instance that all
filesystems could reference, but ->writepages() is fundamentally an
address_space_operation. Until we can rework that I'd prefer the
overhead of the extra exports than sprinkling more IS_DAX checks
around.