linux-kernel - Re: [PATCH v5 02/11] xfs, dax: introduce xfs_dax

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPcyv4iZFW9Lk=KdEy=0iYzM9GmQ4ccoN2imYNDq6XwXDgXsDw@mail.gmail.com>
Date:   Sat, 10 Mar 2018 09:40:03 -0800
From:   Dan Williams <dan.j.williams@...el.com>
To:     Christoph Hellwig <hch@....de>
Cc:     linux-nvdimm <linux-nvdimm@...ts.01.org>,
        Jeff Moyer <jmoyer@...hat.com>,
        Matthew Wilcox <mawilcox@...rosoft.com>,
        Ross Zwisler <ross.zwisler@...ux.intel.com>,
        Jan Kara <jack@...e.cz>, Dave Chinner <david@...morbit.com>,
        linux-xfs <linux-xfs@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 02/11] xfs, dax: introduce xfs_dax_aops

On Sat, Mar 10, 2018 at 1:46 AM, Christoph Hellwig <hch@....de> wrote:
>> +int dax_set_page_dirty(struct page *page)
>> +{
>> +     /*
>> +      * Unlike __set_page_dirty_no_writeback that handles dirty page
>> +      * tracking in the page object, dax does all dirty tracking in
>> +      * the inode address_space in response to mkwrite faults. In the
>> +      * dax case we only need to worry about potentially dirty CPU
>> +      * caches, not dirty page cache pages to write back.
>> +      *
>> +      * This callback is defined to prevent fallback to
>> +      * __set_page_dirty_buffers() in set_page_dirty().
>> +      */
>> +     return 0;
>> +}
>
> Make this a generic noop_set_page_dirty maybe?
>
>> +EXPORT_SYMBOL(dax_set_page_dirty);
>> +
>> +void dax_invalidatepage(struct page *page, unsigned int offset,
>> +             unsigned int length)
>> +{
>> +     /*
>> +      * There is no page cache to invalidate in the dax case, however
>> +      * we need this callback defined to prevent falling back to
>> +      * block_invalidatepage() in do_invalidatepage().
>> +      */
>> +}
>
> Same here.

I guess I'm not sure what you mean. These nops are specific to dax I
don't think they make sense in another context besides dax.

>
>> +EXPORT_SYMBOL(dax_invalidatepage);
>
> And EXPORT_SYMBOL_GPL for anything dax-related, please.
>
>> +const struct address_space_operations xfs_dax_aops = {
>> +     .writepages             = xfs_vm_writepages,
>
> Please split out the DAX case from xfs_vm_writepages.

Will do.

> This patch should probably also split into VFS and XFS parts.

Ok.