Closed
Conversation
Looking at the BIO_CHAIN flag it seems that it appear in the bio after splitting it. Below is presented real life example. First bio with timestamp 78386.890436 is splitted into two: first with 2048 sectors which is managed by the Linux kernel and second with timestamp 78387.427706 which size is equal to 6144 sectors. The second is send to our driver with BIO_CHAIN flag. Next this bio is splitted into two: first with 2048 sectors is managed by kernel and second with timestamp 78387.693920 and size 4096 sectors is send to our driver with BIO_CHAIN flag. So one can see the pattern. First not fragmented bio is managed by our driver so we do not need to care about fragments with BIO_CHAIN flag set. [78386.890436] datto: REQ_OP_WRITE <> BIO flags: BIO_TRACE_COMPLETION BIO_REMAPPED BLK status: BLK_STS_OK s: 34069984-34078176 8192 [78387.427706] datto: REQ_OP_WRITE <> BIO flags: BIO_CHAIN BIO_TRACE_COMPLETION BIO_REMAPPED BLK status: BLK_STS_OK s: 34072032-34078176 6144 [78387.693920] datto: REQ_OP_WRITE <> BIO flags: BIO_CHAIN BIO_TRACE_COMPLETION BIO_REMAPPED BLK status: BLK_STS_OK s: 34074080-34078176 4096 [78388.159220] datto: REQ_OP_WRITE <> BIO flags: BIO_CHAIN BIO_TRACE_COMPLETION BIO_REMAPPED BLK status: BLK_STS_OK s: 34076128-34078176 2048 The solution that will solve some issues related with bio looping inside the dattobd include omitting requests with BIO_CHAIN flag set. As described above, the bio which include chained fragments is managed entirely (all sectors) at once in dattobd so we do not care about fragments. If we care then it will make a disaster on some distributions because of too big amount of memory usage. The DATTOBD_PASSTHROUGH flag has been used to mark already managed bio. When one distinguish if the bio need to be managed by the COW manager using BIO_CHAIN flag then there is no purpose to use this flag. Additionally the system enum type has been used to mark bio with DATTOBD_PASSTHROUGH which seems to be not a good solution.
krzysztof-wasieczko
approved these changes
Jan 12, 2026
AntonRamanenka
suggested changes
Jan 15, 2026
Comment on lines
+1403
to
+1409
| smp_rmb(); | ||
| tracer_for_each(dev, i) | ||
| // The original bio which include (or will include) chained fragments | ||
| // is managed entirely (all sectors) at once by the COW manager. Hence | ||
| // we must not care about chained fragments. | ||
| if (!bio_flagged(bio, BIO_CHAIN)) | ||
| { | ||
| smp_rmb(); | ||
| tracer_for_each(dev, i) |
There was a problem hiding this comment.
I would suggest to keep order of operations the same to avoid introducing new bugs related to race conditions because of the moved memory barrier. Also, after those changes, orig_fn in most of the cases will be NULL and code just below if(unlikely(orig_fn == NULL)) becomes not fully correct (with unlikely).
Author
There was a problem hiding this comment.
Thanks for the comment. Variable orig_fn will be NULL only when BIO_CHAIN flag is set (and this is rare case). However I have introduced some changes accordingly.
Author
|
The approach to solve the issue has changed. New PR is going to be prepared. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Looking at the BIO_CHAIN flag it seems that it appear in the bio after splitting it. Below is presented real life example. First bio with timestamp 78386.890436 is splitted into two: first with 2048 sectors which is managed by the Linux kernel and second with timestamp 78387.427706 which size is equal to 6144 sectors. The second is send to our driver with BIO_CHAIN flag. Next this bio is splitted into two: first with 2048 sectors is managed by kernel and second with timestamp 78387.693920 and size 4096 sectors is send to our driver with BIO_CHAIN flag. So one can see the pattern. First not fragmented bio is managed by our driver so we do not need to care about fragments with BIO_CHAIN flag set.
[78386.890436] datto: REQ_OP_WRITE <> BIO flags: BIO_TRACE_COMPLETION BIO_REMAPPED BLK status: BLK_STS_OK s: 34069984-34078176 8192
[78387.427706] datto: REQ_OP_WRITE <> BIO flags: BIO_CHAIN BIO_TRACE_COMPLETION BIO_REMAPPED BLK status: BLK_STS_OK s: 34072032-34078176 6144
[78387.693920] datto: REQ_OP_WRITE <> BIO flags: BIO_CHAIN BIO_TRACE_COMPLETION BIO_REMAPPED BLK status: BLK_STS_OK s: 34074080-34078176 4096
[78388.159220] datto: REQ_OP_WRITE <> BIO flags: BIO_CHAIN BIO_TRACE_COMPLETION BIO_REMAPPED BLK status: BLK_STS_OK s: 34076128-34078176 2048
The solution that will solve some issues related with bio looping inside the dattobd include omitting requests with BIO_CHAIN flag set. As described above, the bio which include chained fragments is managed entirely (all sectors) at once in dattobd so we do not care about fragments. If we care then it will make a disaster on some distributions because of too big amount of memory usage.
The DATTOBD_PASSTHROUGH flag has been used to mark already managed bio. When one distinguish if the bio need to be managed by the COW manager using BIO_CHAIN flag then there is no purpose to use this flag. Additionally the system enum type has been used to mark bio with DATTOBD_PASSTHROUGH which seems to be not a good solution.