On Tue, May 30, 2023 at 9:25 PM Christoph Hellwig wrote:
>
> To me these look like __bio_add_page candidates, but I guess Song
> preferred it this way? It'll add a bit pointless boilerplate code,
> but I'm ok with that.
We had some discussion on this in v2, and decided to keep these
assert-like
Looks good:
Reviewed-by: Christoph Hellwig
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
On Tue, May 30, 2023 at 08:49:23AM -0700, Johannes Thumshirn wrote:
> +bool __must_check bio_add_folio(struct bio *, struct folio *, size_t len,
> size_t off);
Please spell out the parameters and avoid the overly long line.
--
dm-devel mailing list
dm-devel@redhat.com
On Tue, May 30, 2023 at 08:49:22AM -0700, Johannes Thumshirn wrote:
> When the iomap buffered-io code can't add a folio to a bio, it allocates a
> new bio and adds the folio to that one. This is done using bio_add_folio(),
> but doesn't check for errors.
>
> As adding a folio to a newly created
On Tue, May 30, 2023 at 08:49:21AM -0700, Johannes Thumshirn wrote:
> Just like for bio_add_pages() add a no-fail variant for bio_add_folio().
Can we call this bio_add_folio_nofail? I really regret the __ prefix for
bio_add_page these days - it wasn't really intended to be used as widely
> +int __must_check bio_add_page(struct bio *, struct page *, unsigned len,
> unsigned off);
Please spell out all parameters while you touch this, and also avoid the
overly long line.
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
To me these look like __bio_add_page candidates, but I guess Song
preferred it this way? It'll add a bit pointless boilerplate code,
but I'm ok with that.
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
Looks good:
Reviewed-by: Christoph Hellwig
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
On Tue, May 30, 2023 at 08:49:16AM -0700, Johannes Thumshirn wrote:
> alloc_behind_master_bio() can possibly add multiple pages to a bio, but it
> is not checking for the return value of bio_add_page() if adding really
> succeeded.
>
> Check if the page adding succeeded and if not bail out.
>
>
Looks good:
Reviewed-by: Christoph Hellwig
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
Looks good:
Reviewed-by: Christoph Hellwig
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
Looks good:
Reviewed-by: Christoph Hellwig
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
Looks good:
Reviewed-by: Christoph Hellwig
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
Looks good:
Reviewed-by: Christoph Hellwig
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
Looks good:
Reviewed-by: Christoph Hellwig
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
Looks good:
Reviewed-by: Christoph Hellwig
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
Looks good:
Reviewed-by: Christoph Hellwig
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
Looks good:
Reviewed-by: Christoph Hellwig
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
On Tue, May 30, 2023 at 05:00:39PM -0400, Mikulas Patocka wrote:
> I'd like to know how do you want to do coverage analysis? By instrumenting
> each branch and creating a test case that tests that the branch goes both
> ways?
Documentation/dev-tools/gcov.rst. The compiler instruments each
On 5/23/23 18:40, Eric Biggers wrote:
On Tue, May 23, 2023 at 05:45:00PM -0400, J. corwin Coburn wrote:
The dm-vdo target provides inline deduplication, compression, zero-block
elimination, and thin provisioning. A dm-vdo target can be backed by up to
256TB of storage, and can present a logical
On Mon, 29 May 2023, Kent Overstreet wrote:
> On Mon, May 29, 2023 at 04:59:40PM -0400, Mikulas Patocka wrote:
> > Hi
> >
> > I improved the dm-flakey device mapper target, so that it can do random
> > corruption of read and write bios - I uploaded it here:
> >
Documents the meaning of the "buffer", adds documentation of the current
defaults, and provides an example of how the tunables relate to each
other.
Signed-off-by: Russell Harmon
---
.../device-mapper/dm-integrity.rst| 44 ---
1 file changed, 27 insertions(+), 17
Otherwise subsequent code will dereference a misaligned
`struct dm_target_spec *`, which is undefined behavior.
Signed-off-by: Demi Marie Obenour
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: sta...@vger.kernel.org
---
drivers/md/dm-ioctl.c | 7 +++
1 file changed, 7 insertions(+)
diff
This can be used to avoid race conditions in which a device is destroyed
and recreated with the same major/minor, name, or UUID. diskseqs are
only honored if strict parameter checking is on, to avoid any risk of
breaking old userspace.
Signed-off-by: Demi Marie Obenour
---
This allows specifying a disk sequence number in XenStore. If it does
not match the disk sequence number of the underlying device, the device
will not be exported and a warning will be logged. Userspace can use
this to eliminate race conditions due to major/minor number reuse.
Old kernels do not
Set "opened" to "0" before the hotplug script is called. Once the
device node has been opened, set "opened" to "1".
"opened" is used exclusively by userspace. It serves two purposes:
1. It tells userspace that the diskseq Xenstore entry is supported.
2. It tells userspace that it can wait for
This prevents dm_split_args() from corrupting this struct.
Signed-off-by: Demi Marie Obenour
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: sta...@vger.kernel.org
---
drivers/md/dm-ioctl.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
On Tue, May 30, 2023 at 8:50 AM Johannes Thumshirn
wrote:
>
> alloc_behind_master_bio() can possibly add multiple pages to a bio, but it
> is not checking for the return value of bio_add_page() if adding really
> succeeded.
>
> Check if the page adding succeeded and if not bail out.
>
>
Previously the error was "unable to find target", which is not helpful.
Signed-off-by: Demi Marie Obenour
---
drivers/md/dm-ioctl.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index
po 22. 5. 2023 v 13:17 odesílatel Nitesh Shetty napsal:
>
> +static int __blkdev_copy_offload(struct block_device *bdev_in, loff_t pos_in,
> + struct block_device *bdev_out, loff_t pos_out, size_t len,
> + cio_iodone_t endio, void *private, gfp_t gfp_mask)
> +{
> +
The NUL terminator for each target parameter string must preceed the
following 'struct dm_target_spec'. Otherwise, dm_split_args() might
corrupt this struct.
Signed-off-by: Demi Marie Obenour
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: sta...@vger.kernel.org
---
drivers/md/dm-ioctl.c | 32
On 23/05/29 06:55PM, Matthew Wilcox wrote:
On Mon, May 22, 2023 at 04:11:33PM +0530, Nitesh Shetty wrote:
+ token = alloc_page(gfp_mask);
Why is PAGE_SIZE the right size for 'token'? That seems quite unlikely.
I could understand it being SECTOR_SIZE or something that's
This work aims to allow userspace to create and destroy block devices
in a race-free way, and to allow them to be exposed to other Xen VMs via
blkback without races.
Changes since v1:
- Several device-mapper fixes added.
- The diskseq is now a separate Xenstore node, rather than being part of
Especially on 32-bit systems, it is possible for the pointer arithmetic
to overflow and cause a userspace pointer to be dereferenced in the
kernel.
Signed-off-by: Demi Marie Obenour
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: sta...@vger.kernel.org
---
drivers/md/dm-ioctl.c | 19
The version is fetched once in check_version(), which then does some
validation and then overwrites the version in userspace with the API
version supported by the kernel. copy_params() then fetches the version
from userspace *again*, and this time no validation is done. The result
is that the
> > +/*
> > + * @bdev_in: source block device
> > + * @pos_in: source offset
> > + * @bdev_out:destination block device
> > + * @pos_out: destination offset
> > + * @len: length in bytes to be copied
> > + * @endio: endio function to be called on completion of copy operation,
> > +
This adds a couple of BUILD_BUG_ON()s and moves some arithmetic after
the validation code that checks the arithmetic’s preconditions. The
previous code was correct but could potentially trip sanitizers that
check for unsigned integer wraparound.
Signed-off-by: Demi Marie Obenour
---
On Tue, May 30, 2023 at 09:12:44AM +1000, Dave Chinner wrote:
> Perhaps it is worthwhile running the same tests on btrfs so we can
> something to compare the bcachefs behaviour to. I suspect that btrfs
> will fair little better on the single device, no checksums
> corruption test
It's also a
On Mon, May 29, 2023 at 04:59:40PM -0400, Mikulas Patocka wrote:
> Hi
>
> I improved the dm-flakey device mapper target, so that it can do random
> corruption of read and write bios - I uploaded it here:
> https://people.redhat.com/~mpatocka/testcases/bcachefs/dm-flakey.c
>
> I set up
Currently, device-mapper ioctls ignore unknown flags. This makes
adding new flags to a given ioctl risky, as it could potentially break
old userspace.
To solve this problem, allow userspace to pass 5 as the major version to
any ioctl. This causes the kernel to reject any flags that are not
Userspace can use this to avoid spamming udev with events that udev
should ignore.
Signed-off-by: Demi Marie Obenour
---
drivers/md/dm-core.h | 2 +
drivers/md/dm-ioctl.c | 78 ++-
drivers/md/dm.c | 5 ++-
Typical userspace setups create a symlink under /dev/mapper with the
name of the device, but /dev/mapper/control is reserved for the control
device. Therefore, trying to create such a device is almost certain to
be a userspace bug.
Signed-off-by: Demi Marie Obenour
---
drivers/md/dm-ioctl.c |
On 23/05/30 01:29PM, Maurizio Lombardi wrote:
po 22. 5. 2023 v 13:17 odesílatel Nitesh Shetty napsal:
+static int __blkdev_copy_offload(struct block_device *bdev_in, loff_t pos_in,
+ struct block_device *bdev_out, loff_t pos_out, size_t len,
+ cio_iodone_t endio,
Not only is this helpful for debugging, it also saves the caller an
ioctl in the case where a device should be used if it exists or created
otherwise. To ensure existing userspace is not broken, this feature is
only enabled in strict mode.
Signed-off-by: Demi Marie Obenour
---
Using either of these is going to greatly confuse userspace, as they are
not valid symlink names and so creating the usual /dev/mapper/NAME
symlink will not be possible. As creating a device with either of these
names is almost certainly a userspace bug, just error out.
Signed-off-by: Demi Marie
>
> The buffer_head submission code uses bio_add_page() to add a page to a
> newly created bio. bio_add_page() can fail, but the return value is never
> checked.
>
> Use __bio_add_page() as adding a single page to a newly created bio is
> guaranteed to succeed.
>
> This brings us a step closer
The previous patch for checking diskseq in blkback is not enough to
prevent the following race:
1. Program X opens a loop device
2. Program X gets the diskseq of the loop device.
3. Program X associates a file with the loop device.
4. Program X passes the loop device major, minor, and diskseq to
On 5/26/2023 12:23 AM, Eric Biggers wrote:
On Wed, May 24, 2023 at 09:57:17AM -0700, Chang S. Bae wrote:
== API Limitation ==
The setkey() function transforms an AES key into a handle. But, an
extended key is a usual outcome of setkey() in other AES cipher
implementations. For this reason, a
On 5/25/2023 11:54 PM, Eric Biggers wrote:
On Wed, May 24, 2023 at 09:57:15AM -0700, Chang S. Bae wrote:
+static inline unsigned long aes_align_addr(unsigned long addr)
+{
+ return (crypto_tfm_ctx_alignment() >= AESNI_ALIGN) ?
+ ALIGN(addr, 1) : ALIGN(addr, AESNI_ALIGN);
+}
On Tue, 30 May 2023, Mike Snitzer wrote:
> On Tue, May 30 2023 at 11:13P -0400,
> Mikulas Patocka wrote:
>
> > Hi
> >
> > I nack this. This just adds code that can't ever be executed.
> >
> > dm-crypt already allocates enough entries in the vector (see "unsigned int
> > nr_iovecs = (size
On 30.05.23 18:10, Mike Snitzer wrote:
> On Tue, May 30 2023 at 11:49P -0400,
> Johannes Thumshirn wrote:
>
>> Check if adding pages to clone bio fails and if it does retry with
>> reclaim. This mirrors the behaviour of page allocation in
>> crypt_alloc_buffer().
>
> Nope.
>
>> This way we can
On Tue, May 30 2023 at 11:49P -0400,
Johannes Thumshirn wrote:
> Check if adding pages to clone bio fails and if it does retry with
> reclaim. This mirrors the behaviour of page allocation in
> crypt_alloc_buffer().
Nope.
> This way we can mark bio_add_pages as __must_check.
>
> Reviewed-by:
On Tue, May 30, 2023 at 08:49:23AM -0700, Johannes Thumshirn wrote:
> Now that all callers of bio_add_folio() check the return value, mark it as
> __must_check.
>
> Signed-off-by: Johannes Thumshirn
Reviewed-by: Matthew Wilcox (Oracle)
--
dm-devel mailing list
dm-devel@redhat.com
On Tue, May 30, 2023 at 08:49:22AM -0700, Johannes Thumshirn wrote:
> When the iomap buffered-io code can't add a folio to a bio, it allocates a
> new bio and adds the folio to that one. This is done using bio_add_folio(),
> but doesn't check for errors.
>
> As adding a folio to a newly created
On Tue, May 30, 2023 at 08:49:21AM -0700, Johannes Thumshirn wrote:
> Just like for bio_add_pages() add a no-fail variant for bio_add_folio().
>
> Signed-off-by: Johannes Thumshirn
Reviewed-by: Matthew Wilcox (Oracle)
--
dm-devel mailing list
dm-devel@redhat.com
Just like for bio_add_pages() add a no-fail variant for bio_add_folio().
Signed-off-by: Johannes Thumshirn
---
block/bio.c | 8
include/linux/bio.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/block/bio.c b/block/bio.c
index 043944fd46eb..350c653d4a57 100644
---
Check if adding pages to resync bio fails and if bail out.
As the comment above suggests this cannot happen, WARN if it actually
happens.
This way we can mark bio_add_pages as __must_check.
Reviewed-by: Damien Le Moal
Acked-by: Song Liu
Signed-off-by: Johannes Thumshirn
---
When the iomap buffered-io code can't add a folio to a bio, it allocates a
new bio and adds the folio to that one. This is done using bio_add_folio(),
but doesn't check for errors.
As adding a folio to a newly created bio can't fail, use the newly
introduced __bio_add_folio() function.
Now that all users of bio_add_page check for the return value, mark
bio_add_page as __must_check.
Reviewed-by: Damien Le Moal
Signed-off-by: Johannes Thumshirn
---
include/linux/bio.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/bio.h b/include/linux/bio.h
Now that all callers of bio_add_folio() check the return value, mark it as
__must_check.
Signed-off-by: Johannes Thumshirn
---
include/linux/bio.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 4232a17e6b10..fef9f3085a02
The sync request code uses bio_add_page() to add a page to a newly created bio.
bio_add_page() can fail, but the return value is never checked.
Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.
This brings us a step closer to marking bio_add_page() as
The JFS IO code uses bio_add_page() to add a page to a newly created bio.
bio_add_page() can fail, but the return value is never checked.
Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.
This brings us a step closer to marking bio_add_page() as
Check if adding pages to clone bio fails and if it does retry with
reclaim. This mirrors the behaviour of page allocation in
crypt_alloc_buffer().
This way we can mark bio_add_pages as __must_check.
Reviewed-by: Damien Le Moal
Signed-off-by: Johannes Thumshirn
---
drivers/md/dm-crypt.c | 5
The raid5-ppl submission code uses bio_add_page() to add a page to a
newly created bio. bio_add_page() can fail, but the return value is never
checked. For adding consecutive pages, the return is actually checked and
a new bio is allocated if adding the page fails.
Use __bio_add_page() as adding
alloc_behind_master_bio() can possibly add multiple pages to a bio, but it
is not checking for the return value of bio_add_page() if adding really
succeeded.
Check if the page adding succeeded and if not bail out.
Reviewed-by: Damien Le Moal
Signed-off-by: Johannes Thumshirn
---
The floppy code uses bio_add_page() to add a page to a newly created bio.
bio_add_page() can fail, but the return value is never checked.
Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.
This brings us a step closer to marking bio_add_page() as
The zram writeback code uses bio_add_page() to add a page to a newly
created bio. bio_add_page() can fail, but the return value is never
checked.
Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.
This brings us a step closer to marking bio_add_page()
The zonefs superblock reading code uses bio_add_page() to add a page to a
newly created bio. bio_add_page() can fail, but the return value is
never checked.
Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.
This brings us a step closer to marking
The md-raid superblock writing code uses bio_add_page() to add a page to a
newly created bio. bio_add_page() can fail, but the return value is never
checked.
Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.
This brings us a step closer to marking
The GFS2 superblock reading code uses bio_add_page() to add a page to a
newly created bio. bio_add_page() can fail, but the return value is never
checked.
Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.
This brings us a step closer to marking
dm-zoned uses bio_add_page() for adding a single page to a freshly created
metadata bio.
Use __bio_add_page() instead as adding a single page to a new bio is
always guaranteed to succeed.
This brings us a step closer to marking bio_add_page() __must_check
Reviewed-by: Damien Le Moal
The drbd code only adds a single page to a newly created bio. So use
__bio_add_page() to add the page which is guaranteed to succeed in this
case.
This brings us closer to marking bio_add_page() as __must_check.
Reviewed-by: Damien Le Moal
Signed-off-by: Johannes Thumshirn
---
The raid5 log metadata submission code uses bio_add_page() to add a page
to a newly created bio. bio_add_page() can fail, but the return value is
never checked.
Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.
This brings us a step closer to marking
The buffer_head submission code uses bio_add_page() to add a page to a
newly created bio. bio_add_page() can fail, but the return value is never
checked.
Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.
This brings us a step closer to marking
On Tue, May 30 2023 at 11:13P -0400,
Mikulas Patocka wrote:
>
>
> On Tue, 2 May 2023, Johannes Thumshirn wrote:
>
> > Check if adding pages to clone bio fails and if it does retry with
> > reclaim. This mirrors the behaviour of page allocation in
> > crypt_alloc_buffer().
> >
> > This way we
We have two functions for adding a page to a bio, __bio_add_page() which is
used to add a single page to a freshly created bio and bio_add_page() which is
used to add a page to an existing bio.
While __bio_add_page() is expected to succeed, bio_add_page() can fail.
This series converts the
The swap code only adds a single page to a newly created bio. So use
__bio_add_page() to add the page which is guaranteed to succeed in this
case.
This brings us closer to marking bio_add_page() as __must_check.
Reviewed-by: Damien Le Moal
Signed-off-by: Johannes Thumshirn
---
mm/page_io.c |
On Tue, May 30 2023 at 10:55P -0400,
Joe Thornber wrote:
> On Tue, May 30, 2023 at 3:02 PM Mike Snitzer wrote:
>
> >
> > Also Joe, for you proposed dm-thinp design where you distinquish
> > between "provision" and "reserve": Would it make sense for REQ_META
> > (e.g. all XFS metadata) with
On 5/26/23 12:37 AM, Johannes Thumshirn wrote:
> On 24.05.23 17:02, Jens Axboe wrote:
>> On 5/2/23 4:19?AM, Johannes Thumshirn wrote:
>>> We have two functions for adding a page to a bio, __bio_add_page() which is
>>> used to add a single page to a freshly created bio and bio_add_page() which
>>>
On Tue, 2 May 2023, Johannes Thumshirn wrote:
> Check if adding pages to clone bio fails and if it does retry with
> reclaim. This mirrors the behaviour of page allocation in
> crypt_alloc_buffer().
>
> This way we can mark bio_add_pages as __must_check.
>
> Reviewed-by: Damien Le Moal
>
On Tue, May 30, 2023 at 3:02 PM Mike Snitzer wrote:
>
> Also Joe, for you proposed dm-thinp design where you distinquish
> between "provision" and "reserve": Would it make sense for REQ_META
> (e.g. all XFS metadata) with REQ_PROVISION to be treated as an
> LBA-specific hard request? Whereas
On Tue, May 30 2023 at 3:27P -0400,
Joe Thornber wrote:
> On Sat, May 27, 2023 at 12:45 AM Dave Chinner wrote:
>
> > On Fri, May 26, 2023 at 12:04:02PM +0100, Joe Thornber wrote:
> >
> > > 1) We have an api (ioctl, bio flag, whatever) that lets you
> > > reserve/guarantee a region:
> > >
> >
On Mon, 29 May 2023, Matthew Wilcox wrote:
> On Mon, May 29, 2023 at 04:59:40PM -0400, Mikulas Patocka wrote:
> > Hi
> >
> > I improved the dm-flakey device mapper target, so that it can do random
> > corruption of read and write bios - I uploaded it here:
> >
On Mon, 29 May 2023, Mikulas Patocka wrote:
> The oops happens in set_btree_iter_dontneed and it is caused by the fact
> that iter->path is NULL. The code in try_alloc_bucket is buggy because it
> sets "struct btree_iter iter = { NULL };" and then jumps to the "err"
> label that tries to
On Sat, May 27, 2023 at 12:45 AM Dave Chinner wrote:
> On Fri, May 26, 2023 at 12:04:02PM +0100, Joe Thornber wrote:
>
> > 1) We have an api (ioctl, bio flag, whatever) that lets you
> > reserve/guarantee a region:
> >
> > int reserve_region(dev, sector_t begin, sector_t end);
>
> A C-based
85 matches
Mail list logo
