Re: [PATCH 2/7] brd: convert to folios

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/14/23 15:45, Matthew Wilcox wrote:
On Wed, Jun 14, 2023 at 01:46:32PM +0200, Hannes Reinecke wrote:
  /*
- * Each block ramdisk device has a xarray brd_pages of pages that stores
- * the pages containing the block device's contents. A brd page's ->index is
- * its offset in PAGE_SIZE units. This is similar to, but in no way connected
- * with, the kernel's pagecache or buffer cache (which sit above our block
- * device).
+ * Each block ramdisk device has a xarray of folios that stores the folios
+ * containing the block device's contents. A brd folio's ->index is its offset
+ * in PAGE_SIZE units. This is similar to, but in no way connected with,
+ * the kernel's pagecache or buffer cache (which sit above our block device).

Having read my way to the end of the series, I can now circle back and
say this comment is wrong.  The folio->index is its offset in PAGE_SIZE
units if the sector size is <= PAGE_SIZE, otherwise it's the offset in
sector size units.  This is _different from_ the pagecache which uses
PAGE_SIZE units and multi-index entries in the XArray.

Hmm. I am aware that brd is using the ->index field in a different way,
but I thought I got it straigtened out ...

@@ -144,29 +143,29 @@ static int copy_to_brd_setup(struct brd_device *brd, sector_t sector, size_t n,
  static void copy_to_brd(struct brd_device *brd, const void *src,
  			sector_t sector, size_t n)
  {
-	struct page *page;
+	struct folio *folio;
  	void *dst;
  	unsigned int offset = (sector & (PAGE_SECTORS-1)) << SECTOR_SHIFT;
  	size_t copy;
copy = min_t(size_t, n, PAGE_SIZE - offset);
-	page = brd_lookup_page(brd, sector);
-	BUG_ON(!page);
+	folio = brd_lookup_folio(brd, sector);
+	BUG_ON(!folio);
- dst = kmap_atomic(page);
-	memcpy(dst + offset, src, copy);
-	kunmap_atomic(dst);
+	dst = kmap_local_folio(folio, offset);
+	memcpy(dst, src, copy);
+	kunmap_local(dst);

This should use memcpy_to_folio(), which doesn't exist yet.
Compile-tested patch incoming shortly ...

Ah. You live and learn.

+	folio = brd_lookup_folio(brd, sector);
+	if (folio) {
+		src = kmap_local_folio(folio, offset);
+		memcpy(dst, src, copy);
+		kunmap_local(src);

And this will need memcpy_from_folio(), patch for that incoming too.

@@ -226,15 +225,15 @@ static int brd_do_bvec(struct brd_device *brd, struct page *page,
  			goto out;
  	}
- mem = kmap_atomic(page);
+	mem = kmap_local_folio(folio, off);
  	if (!op_is_write(opf)) {
-		copy_from_brd(mem + off, brd, sector, len);
-		flush_dcache_page(page);
+		copy_from_brd(mem, brd, sector, len);
+		flush_dcache_folio(folio);

Nngh.  This will need to be a more complex loop.  I don't think we can
do a simple abstraction here.  Perhaps you can base it on the two
patches you're about to see?

Sure.
Might explain the strange crashes I've seen ...

Cheers,

Hannes




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux