[ABD] generic abd in vdev_label.c #5555
Conversation
|
@dpquigl, thanks for your PR! By analyzing the history of the files in this pull request, we identified @grwilson, @behlendorf and @tonyhutter to be potential reviewers. |
|
Can you elaborate why you want to change |
|
@tuxoko I think the reason for this is that in the future we may change the abd_alloc_for_io implementation to something that works better on Linux. The comment in the code says for now its a linear abd but that could change if certain components change their interfaces. |
|
From the comment, it seems that this API exists because illumos can only issue linear buffer in the block IO stack. That's not the case in Linux, we only care about the extra copy overhead of the buffer user. This should be solved in a case by case basis, and changing all linear allocation to |
There was a problem hiding this comment.
From the comment, it seems that this API exists because illumos can only issue linear buffer in the block IO stack. That's not the case in Linux.
Yes, that's right. The intention here is to eventually replace the _for_io() function with on that does the right thing for Linux or Illumos. That means creating a scatter abd for Linux and a linear one for illumos, in order to benefit from this we need to get rid of the existing _linear() consumers which is what this patch does.
| err = nvlist_unpack(vp->vp_nvlist, | ||
| sizeof (vp->vp_nvlist), &label, 0); | ||
|
|
||
| abd_return_buf_copy(vp_abd, vp, sizeof (vdev_phys_t)); |
There was a problem hiding this comment.
It's a little thing but let's remove the extra whitespace above.
| @@ -934,19 +941,22 @@ vdev_label_init(vdev_t *vd, uint64_t crtxg, vdev_labeltype_t reason) | |||
| error = nvlist_pack(label, &buf, &buflen, NV_ENCODE_XDR, KM_SLEEP); | |||
| if (error != 0) { | |||
| nvlist_free(label); | |||
| abd_return_buf_copy(vp_abd, vp, sizeof (vdev_phys_t)); | |||
There was a problem hiding this comment.
This could be abd_return_buf(). There's no reason to copy the borrowed buffer data back in to the abd if we're just going to immediately free it.
behlendorf
changed the title
Signed-off-by: Gvozden Neskovic <neskovic@gmail.com>
|
Closing. There's no compelling reason to make this change in the label code which is called relatively infrequently. |
This is a rebased version of Gvozden Neskovic's patch to remove linear buffer usage in vdev_label converting them to abd structures instead.