linux-kernel - Re: [PATCH v4 4/9] dax: fix mapping lifetime handling, convert to __pfn_t + kmap_atomic_pfn

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] [day] [month] [year] [list]

Message-ID: <CAPcyv4hiz1sFgyZfzJ5MNfuoqy0APZ+PwMG4FXJBz_p-gvoorg@mail.gmail.com>
Date:	Fri, 7 Aug 2015 16:54:07 -0700
From:	Dan Williams <dan.j.williams@...el.com>
To:	Matthew Wilcox <willy@...ux.intel.com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Jens Axboe <axboe@...nel.dk>,
	Boaz Harrosh <boaz@...xistor.com>, david <david@...morbit.com>,
	linux-arch@...r.kernel.org, Arnd Bergmann <arnd@...db.de>,
	"linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>,
	Heiko Carstens <heiko.carstens@...ibm.com>,
	Christoph Hellwig <hch@....de>, Ingo Molnar <mingo@...nel.org>,
	Tejun Heo <tj@...nel.org>, Paul Mackerras <paulus@...ba.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	Martin Schwidefsky <schwidefsky@...ibm.com>,
	Ross Zwisler <ross.zwisler@...ux.intel.com>,
	"torvalds@...ux-foundation.org" <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH v4 4/9] dax: fix mapping lifetime handling, convert to
 __pfn_t + kmap_atomic_pfn_t()

On Sat, Jun 6, 2015 at 4:58 AM, Matthew Wilcox <willy@...ux.intel.com> wrote:
> On Fri, Jun 05, 2015 at 05:19:24PM -0400, Dan Williams wrote:
>> @@ -35,13 +35,16 @@ int dax_clear_blocks(struct inode *inode, sector_t block, long size)
>>       might_sleep();
>>       do {
>>               void *addr;
>> -             unsigned long pfn;
>> +             __pfn_t pfn;
>>               long count;
>>
>> -             count = bdev_direct_access(bdev, sector, &addr, &pfn, size);
>> +             count = bdev_direct_access(bdev, sector, &pfn, size);
>>               if (count < 0)
>>                       return count;
>>               BUG_ON(size < count);
>> +             addr = kmap_atomic_pfn_t(pfn);
>> +             if (!addr)
>> +                     return -EIO;
>>               while (count > 0) {
>>                       unsigned pgsz = PAGE_SIZE - offset_in_page(addr);
>>                       if (pgsz > count)
>
> This part is incomplete.  When bdev_direct_access() could return an
> address, it was possible for that address to be unaligned (eg when
> 'sector' was not a multiple of 8).  DAX has never had full support for
> devices that weren't a 4k sector size, but I was trying to not make that
> assumption in more places than I had to.  So this function needs a lot
> more simplification (or it needs to add '(sector & 7) << 9' to addr ...
> assuming that the partition this bdev represents actually starts at a
> multiple of 8 ... bleh!).

Isn't this already handled by the:

    if (sector % (PAGE_SIZE / 512))
                    return -EINVAL;

...check in bdev_direct_access()?  As long as the driver's mapping is
4K aligned, which appears to be the case for all DAX-enabled drivers,
then we should be good to go.

>>
>> -static long dax_get_addr(struct buffer_head *bh, void **addr, unsigned blkbits)
>> +static long dax_get_pfn(struct buffer_head *bh, __pfn_t *pfn, unsigned blkbits)
>>  {
>> -     unsigned long pfn;
>>       sector_t sector = bh->b_blocknr << (blkbits - 9);
>> -     return bdev_direct_access(bh->b_bdev, sector, addr, &pfn, bh->b_size);
>> +     return bdev_direct_access(bh->b_bdev, sector, pfn, bh->b_size);
>>  }
>
> This function should just be deleted.  It offers essentially nothing
> over just calling bdev_direct_access().

Ok.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/