Linux 4.8 compat #4951

behlendorf · 2016-08-09T21:36:02Z

Linux 4.8 compatibility and a minor bit of unrelated code cleanup.

behlendorf · 2016-08-09T21:38:15Z

@tuxoko can I get your thoughts on this. It resolves the same issue you opened #4949 for but I took the opportunity to do a little additional cleanup and refactoring to keep this code maintainable.

tuxoko · 2016-08-09T22:09:23Z

include/linux/blkdev_compat.h

 static inline void
-__blk_queue_write_cache(struct request_queue *q, bool wc, bool fua)
+blk_queue_set_write_cache(struct request_queue *q, bool wc, bool fua)
 {
 	spin_lock_irq(q->queue_lock);


Umm... I don't think putting every implementation inside spin_lock is a good idea.

The spinlock does exist all the way back to 2.6.32 but your right it is a small functional change, which I'm pretty sure is safe. But I wouldn't object to just moving it back to the one case.

Oh. I take it back. That'll deadlock for the case blk_queue_write_cache() exists and we can use it. Will fix.

tuxoko · 2016-08-09T22:14:57Z

@behlendorf The overall cleanup looks pretty good. Though there are some details that need to get figured out.

Non-Linux OpenZFS implementations require additional support to be used a root pool. This code should simply be removed to avoid confusion and improve readability. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>

behlendorf · 2016-08-09T22:29:53Z

Refreshed with the spin lock fix.

tuxoko · 2016-08-09T22:46:06Z

https://github.com/torvalds/linux/blob/master/Documentation/block/writeback_cache_control.txt#L50

We are using make_request_fn based implementation, so we will get REQ_PREFLUSH, which means flush before write.

tuxoko · 2016-08-09T22:56:55Z

include/linux/blkdev_compat.h

-#if defined(HAVE_BIO_RW_DISCARD)
-	return (bio->bi_rw & (1 << BIO_RW_DISCARD));
+#if defined(HAVE_REQ_OP_DISCARD)
+	return (bio_op(bio) == REQ_OP_DISCARD);


Since we have REQ_OP_SECURE_ERASE, we should check both.

I don't follow, this check should only concern itself with HAVE_REQ_OP_DISCARD

Then you should add bio_is_secure_erase to zvol_request where it calls zvol_discard.

tuxoko · 2016-08-09T22:57:24Z

module/zfs/zvol.c

@@ -821,14 +817,14 @@ zvol_request(struct request_queue *q, struct bio *bio)
 		 * Some requests are just for flush and nothing else.
 		 */
 		if (uio.uio_resid == 0) {
-			if (bio->bi_rw & VDEV_REQ_FLUSH)
+			if (bio_is_flush(bio))


Here we need REQ_PREFLUSH.

In order having to restructure this code for multiple kernel I extended the bio_is_flush() check to cover both REQ_PREFLUSH and REQ_OP_FLUSH in newer kernels. This isn't always 100% optimal since we might do an extra flush but it should be functionally correct and keep the code simpler. We can always optimize this if its an issue.

behlendorf · 2016-08-09T22:57:32Z

We are using make_request_fn based implementation, so we will get REQ_PREFLUSH, which means flush before write.

What do you suggest.

tuxoko · 2016-08-09T22:58:48Z

module/zfs/zvol.c

 				zil_commit(zv->zv_zilog, ZVOL_OBJ);
 			goto out2;
 		}

 		error = zvol_write(zv, &uio,
-		    ((bio->bi_rw & (VDEV_REQ_FUA|VDEV_REQ_FLUSH)) ||
-		    zv->zv_objset->os_sync == ZFS_SYNC_ALWAYS));
+		    bio_is_flush(bio) || bio_is_fua(bio) ||


Here we don't need REQ_PREFLUSH, though I'm not sure about REQ_OP_FLUSH, since from document it seems we won't get REQ_OP_FLUSH.

We don't need REQ_PREFLUSH here but it won't be harmful. For simplify I've left the check.

All users of bio->bi_rw have been replaced with compatibility wrappers. This allows the kernel specific logic to be abstracted away, and for each of the supported cases to be documented with the wrapper. The updated interfaces are as follows: * void blk_queue_set_write_cache(struct request_queue *, bool, bool) * boolean_t bio_is_flush(struct bio *) * boolean_t bio_is_fua(struct bio *) * boolean_t bio_is_discard(struct bio *) * boolean_t bio_is_secure_erase(struct bio *) * VDEV_WRITE_FLUSH_FUA Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>

behlendorf · 2016-08-10T20:54:30Z

@tuxoko I've refreshed the PRs with your feedback.

tuxoko · 2016-08-10T21:06:08Z

LGTM as long as it builds fine.

Non-Linux OpenZFS implementations require additional support to be used a root pool. This code should simply be removed to avoid confusion and improve readability. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Closes #4951

The HAVE_BIO_RW_* #ifdef's must appear before REQ_* #ifdef's in the bio_is_flush() and bio_is_discard() macros. Linux 2.6.32 era kernels defined both of values and the HAVE_BIO_RW_* must be used in this case. This resulted in a panic in zconfig test 5. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#4951

The HAVE_BIO_RW_* #ifdef's must appear before REQ_* #ifdef's in the bio_is_flush() and bio_is_discard() macros. Linux 2.6.32 era kernels defined both of values and the HAVE_BIO_RW_* must be used in this case. This resulted in a panic in zconfig test 5. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Closes openzfs#4951 Closes openzfs#4959

thegreatgazoo · 2016-08-31T03:42:44Z

module/zfs/spa.c

- *	"/pci@1f,0/ide@d/disk@0,0:a"
- */
-int
-spa_import_rootpool(char *devpath, char *devid)


Function declaration still exists in include/sys/spa.h.

All users of bio->bi_rw have been replaced with compatibility wrappers. This allows the kernel specific logic to be abstracted away, and for each of the supported cases to be documented with the wrapper. The updated interfaces are as follows: * void blk_queue_set_write_cache(struct request_queue *, bool, bool) * boolean_t bio_is_flush(struct bio *) * boolean_t bio_is_fua(struct bio *) * boolean_t bio_is_discard(struct bio *) * boolean_t bio_is_secure_erase(struct bio *) * VDEV_WRITE_FLUSH_FUA Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Closes openzfs#4951

The HAVE_BIO_RW_* #ifdef's must appear before REQ_* #ifdef's in the bio_is_flush() and bio_is_discard() macros. Linux 2.6.32 era kernels defined both of values and the HAVE_BIO_RW_* must be used in this case. This resulted in a panic in zconfig test 5. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Closes openzfs#4951 Closes openzfs#4959

All users of bio->bi_rw have been replaced with compatibility wrappers. This allows the kernel specific logic to be abstracted away, and for each of the supported cases to be documented with the wrapper. The updated interfaces are as follows: * void blk_queue_set_write_cache(struct request_queue *, bool, bool) * boolean_t bio_is_flush(struct bio *) * boolean_t bio_is_fua(struct bio *) * boolean_t bio_is_discard(struct bio *) * boolean_t bio_is_secure_erase(struct bio *) * VDEV_WRITE_FLUSH_FUA Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Closes openzfs#4951

The HAVE_BIO_RW_* #ifdef's must appear before REQ_* #ifdef's in the bio_is_flush() and bio_is_discard() macros. Linux 2.6.32 era kernels defined both of values and the HAVE_BIO_RW_* must be used in this case. This resulted in a panic in zconfig test 5. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Closes openzfs#4951 Closes openzfs#4959

Non-Linux OpenZFS implementations require additional support to be used a root pool. This code should simply be removed to avoid confusion and improve readability. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Closes openzfs#4951

The HAVE_BIO_RW_* #ifdef's must appear before REQ_* #ifdef's in the bio_is_flush() and bio_is_discard() macros. Linux 2.6.32 era kernels defined both of values and the HAVE_BIO_RW_* must be used in this case. This resulted in a panic in zconfig test 5. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Closes openzfs#4951 Closes openzfs#4959

Non-Linux OpenZFS implementations require additional support to be used a root pool. This code should simply be removed to avoid confusion and improve readability. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Closes openzfs#4951 Requires-builders: style

The HAVE_BIO_RW_* #ifdef's must appear before REQ_* #ifdef's in the bio_is_flush() and bio_is_discard() macros. Linux 2.6.32 era kernels defined both of values and the HAVE_BIO_RW_* must be used in this case. This resulted in a panic in zconfig test 5. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Closes openzfs#4951 Closes openzfs#4959 Requires-builders: style

Non-Linux OpenZFS implementations require additional support to be used a root pool. This code should simply be removed to avoid confusion and improve readability. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Closes #4951

The HAVE_BIO_RW_* #ifdef's must appear before REQ_* #ifdef's in the bio_is_flush() and bio_is_discard() macros. Linux 2.6.32 era kernels defined both of values and the HAVE_BIO_RW_* must be used in this case. This resulted in a panic in zconfig test 5. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Closes #4951 Closes #4959

tuxoko reviewed Aug 9, 2016
View reviewed changes

Remove custom root pool import code

52af1f6

Non-Linux OpenZFS implementations require additional support to be used a root pool. This code should simply be removed to avoid confusion and improve readability. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>

behlendorf force-pushed the linux-4.8-compat branch from 0288a8c to 79fdd7a Compare August 9, 2016 22:29

tuxoko reviewed Aug 9, 2016
View reviewed changes

behlendorf force-pushed the linux-4.8-compat branch from 79fdd7a to 1d162f5 Compare August 9, 2016 22:57

tuxoko reviewed Aug 9, 2016
View reviewed changes

behlendorf force-pushed the linux-4.8-compat branch from 1d162f5 to 26938f3 Compare August 10, 2016 20:51

behlendorf closed this in cf41432 Aug 11, 2016

behlendorf mentioned this pull request Aug 11, 2016

Reorder HAVE_BIO_RW_* checks #4959

Closed

thegreatgazoo reviewed Aug 31, 2016
View reviewed changes

behlendorf deleted the linux-4.8-compat branch May 1, 2017 20:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Linux 4.8 compat #4951

Linux 4.8 compat #4951

behlendorf commented Aug 9, 2016

behlendorf commented Aug 9, 2016

tuxoko Aug 9, 2016

behlendorf Aug 9, 2016

behlendorf Aug 9, 2016

tuxoko commented Aug 9, 2016

behlendorf commented Aug 9, 2016

tuxoko commented Aug 9, 2016

tuxoko Aug 9, 2016

behlendorf Aug 10, 2016

tuxoko Aug 10, 2016

behlendorf Aug 10, 2016

tuxoko Aug 9, 2016

behlendorf Aug 10, 2016

behlendorf commented Aug 9, 2016

tuxoko Aug 9, 2016

behlendorf Aug 10, 2016

behlendorf commented Aug 10, 2016

tuxoko commented Aug 10, 2016

thegreatgazoo Aug 31, 2016

Linux 4.8 compat #4951

Linux 4.8 compat #4951

Conversation

behlendorf commented Aug 9, 2016

behlendorf commented Aug 9, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tuxoko commented Aug 9, 2016

behlendorf commented Aug 9, 2016

tuxoko commented Aug 9, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

behlendorf commented Aug 9, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

behlendorf commented Aug 10, 2016

tuxoko commented Aug 10, 2016

Choose a reason for hiding this comment