linux-kernel - Re: [PATCH 1/2] nvme-hwmon: Cache-line-align the NVME SMART log-buffer

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]

Date:   Mon, 26 Sep 2022 01:23:25 +0300
From:   Serge Semin <fancer.lancer@...il.com>
To:     Christoph Hellwig <hch@....de>
Cc:     Serge Semin <Sergey.Semin@...kalelectronics.ru>,
        Jonathan Derrick <jonathan.derrick@...el.com>,
        Revanth Rajashekar <revanth.rajashekar@...el.com>,
        Jens Axboe <axboe@...nel.dk>, Keith Busch <kbusch@...nel.org>,
        Jens Axboe <axboe@...com>, Sagi Grimberg <sagi@...mberg.me>,
        Guenter Roeck <linux@...ck-us.net>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Pavel Parkhomenko <Pavel.Parkhomenko@...kalelectronics.ru>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        linux-nvme@...ts.infradead.org, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] nvme-hwmon: Cache-line-align the NVME SMART
 log-buffer

Hello Christoph,

Sorry for the delay with response.

On Mon, Sep 12, 2022 at 10:29:10AM +0200, Christoph Hellwig wrote:
> On Sat, Sep 10, 2022 at 03:35:42PM +0300, Serge Semin wrote:
> > Well, both approaches will solve the denoted problem. I am just
> > wondering why do you think that the kmalloc-ed buffer is more
> > preferable?
> 

> Because it clearly documents the intent.  Here is one buffer that is
> just a data buffer, and here is one with kernel internal structure.
> The concept of embedding on-disk / on-the-wire structures into internal
> stuctures always seemed rather weird and unexpected to me, as we now
> need to ensure that the alignment works right on both sides.  With
> the right annotations (as done in this series) this will work, but
> it feels a little fragile to me.

IMO both the approaches seem unclear if a reader doesn't know what
they have been introduced for. Anyway do you insist on using the
kmalloc-ed buffer here instead? If so I'll resubmit the series with
this patch updated accordingly.

-Sergey

> 
> > What would be the best solution if we had a qualifier like this:
> > #ifdef CONFIG_DMA_NONCOHERENT
> > #define ____dma_buffer ____cacheline_aligned
> > #else
> > #define ____dma_buffer
> > #endif
> > and used it instead of the direct ____cacheline_aligned utilization.
> 
> So independent of my preference for separate allocations, this suggested
> additional would still be very useful for the places where we need
> to use the alignment for performance or other reasons.  I'd use
> something like __dma_alligned or similar, though.