Changeset View
Standalone View
sys/kern/vfs_bio.c
Show First 20 Lines • Show All 4,166 Lines • ▼ Show 20 Lines | geteblk(int size, int flags) | ||||
bp->b_flags |= B_INVAL; /* b_dep cleared by getnewbuf() */ | bp->b_flags |= B_INVAL; /* b_dep cleared by getnewbuf() */ | ||||
BUF_ASSERT_HELD(bp); | BUF_ASSERT_HELD(bp); | ||||
return (bp); | return (bp); | ||||
} | } | ||||
/* | /* | ||||
* Truncate the backing store for a non-vmio buffer. | * Truncate the backing store for a non-vmio buffer. | ||||
*/ | */ | ||||
static void | static void | ||||
cem: Ah, by "inline," I had in mind literally moving the code inside the caller.
(The "inline"… | |||||
vfs_nonvmio_truncate(struct buf *bp, int newbsize) | vfs_nonvmio_truncate(struct buf *bp, int newbsize) | ||||
{ | { | ||||
Not Done Inline ActionsThis routine and nonvmio_extend can probably both be inlined now that they're so short (they're each only called in one place). cem: This routine and nonvmio_extend can probably both be inlined now that they're so short (they're… | |||||
if (bp->b_flags & B_MALLOC) { | if (bp->b_flags & B_MALLOC) { | ||||
Not Done Inline ActionsBlank line is needed before the first statement if there is no local vars defined. kib: Blank line is needed before the first statement if there is no local vars defined. | |||||
/* | /* | ||||
Not Done Inline Actionsreturn (false); kib: return (false); | |||||
* malloced buffers are not shrunk | * malloced buffers are not shrunk | ||||
Not Done Inline ActionsNo need for this blank line. kib: No need for this blank line. | |||||
*/ | */ | ||||
if (newbsize == 0) { | if (newbsize == 0) { | ||||
Not Done Inline Actionsreturn (false); kib: return (false); | |||||
bufmallocadjust(bp, 0); | bufmallocadjust(bp, 0); | ||||
Not Done Inline Actionsextra blank line, and so on for the whole function. kib: extra blank line, and so on for the whole function. | |||||
free(bp->b_data, M_BIOBUF); | free(bp->b_data, M_BIOBUF); | ||||
bp->b_data = bp->b_kvabase; | bp->b_data = bp->b_kvabase; | ||||
bp->b_flags &= ~B_MALLOC; | bp->b_flags &= ~B_MALLOC; | ||||
} | } | ||||
return; | return; | ||||
} | } | ||||
newbsize = round_page(newbsize); | |||||
vm_hold_free_pages(bp, newbsize); | vm_hold_free_pages(bp, newbsize); | ||||
bufspace_adjust(bp, newbsize); | bufspace_adjust(bp, newbsize); | ||||
} | } | ||||
/* | /* | ||||
* Extend the backing for a non-VMIO buffer. | * Extend the backing for a non-VMIO buffer. | ||||
*/ | */ | ||||
static void | static void | ||||
vfs_nonvmio_extend(struct buf *bp, int newbsize) | vfs_nonvmio_extend(struct buf *bp, int newbsize) | ||||
{ | { | ||||
caddr_t origbuf; | caddr_t origbuf; | ||||
int origbufsize; | int origbufsize; | ||||
/* | /* | ||||
* We only use malloced memory on the first allocation. | * We only use malloced memory on the first allocation. | ||||
* and revert to page-allocated memory when the buffer | * and revert to page-allocated memory when the buffer | ||||
* grows. | * grows. | ||||
* | * | ||||
* There is a potential smp race here that could lead | * There is a potential smp race here that could lead | ||||
* to bufmallocspace slightly passing the max. It | * to bufmallocspace slightly passing the max. It | ||||
* is probably extremely rare and not worth worrying | * is probably extremely rare and not worth worrying | ||||
* over. | * over. | ||||
*/ | */ | ||||
if (bp->b_bufsize == 0 && newbsize <= PAGE_SIZE/2 && | if (bp->b_bufsize == 0 && newbsize <= PAGE_SIZE/2 && | ||||
bufmallocspace < maxbufmallocspace) { | bufmallocspace < maxbufmallocspace) { | ||||
bp->b_data = malloc(newbsize, M_BIOBUF, M_WAITOK); | bp->b_data = malloc(newbsize, M_BIOBUF, M_WAITOK); | ||||
bp->b_flags |= B_MALLOC; | bp->b_flags |= B_MALLOC; | ||||
bufmallocadjust(bp, newbsize); | bufmallocadjust(bp, newbsize); | ||||
return; | return; | ||||
} | } | ||||
newbsize = round_page(newbsize); | |||||
Not Done Inline ActionsIf we fail the bufmallocspace check here, should we round newbsize up to page_size? Or remove the conditional roundup below? cem: If we fail the bufmallocspace check here, should we round newbsize up to page_size? Or remove… | |||||
Not Done Inline ActionsI'm fine with the roundup if that's the right thing to do. darrick.freebsd_gmail.com: I'm fine with the roundup if that's the right thing to do. | |||||
Not Done Inline Actionsiirc kib explicitly wanted the bufmalloc check removed from want_malloc_buf(). That would've caused the roundup to occur when bufmallocspace >= maxbufmallocspace. darrick.freebsd_gmail.com: iirc kib explicitly wanted the bufmalloc check removed from want_malloc_buf(). That would've… | |||||
Not Done Inline ActionsNo, I do not want to micro-manage. I only want malloc-backed buffer to start/continue to work. Code organization is up to you. kib: No, I do not want to micro-manage. I only want malloc-backed buffer to start/continue to work. | |||||
Not Done Inline ActionsNot a code organization question. Earlier the check in question was in want_malloc_buf(), so the check was done in two places: 1. before rounding up to PAGE_SIZE in the case where we don't want to set B_MALLOC, and 2. prior to setting B_MALLOC in the case where we do. But in an earlier comment you stated that "4291 darrick.freebsd_gmail.com: Not a code organization question. Earlier the check in question was in want_malloc_buf(), so… | |||||
Done Inline ActionsI do not quite understand what you are asking. Which check below ? If we do not allocate malloc buffer, there is no point (and it is even impossible) to allocate not in quantities of PAGE_SIZE. So if the maxbufmallocspace check fails and we decided to allocate page-backed buffer, we need to round. This is the only sense that I can see in the question. kib: I do not quite understand what you are asking. Which check below ?
If we do not allocate… | |||||
/* | /* | ||||
* If the buffer is growing on its other-than-first | * If the buffer is growing on its other-than-first | ||||
* allocation then we revert to the page-allocation | * allocation then we revert to the page-allocation | ||||
* scheme. | * scheme. | ||||
*/ | */ | ||||
origbuf = NULL; | origbuf = NULL; | ||||
origbufsize = 0; | origbufsize = 0; | ||||
if (bp->b_flags & B_MALLOC) { | if (bp->b_flags & B_MALLOC) { | ||||
origbuf = bp->b_data; | origbuf = bp->b_data; | ||||
origbufsize = bp->b_bufsize; | origbufsize = bp->b_bufsize; | ||||
bp->b_data = bp->b_kvabase; | bp->b_data = bp->b_kvabase; | ||||
bufmallocadjust(bp, 0); | bufmallocadjust(bp, 0); | ||||
bp->b_flags &= ~B_MALLOC; | bp->b_flags &= ~B_MALLOC; | ||||
newbsize = round_page(newbsize); | |||||
} | } | ||||
vm_hold_load_pages(bp, (vm_offset_t) bp->b_data + bp->b_bufsize, | vm_hold_load_pages(bp, (vm_offset_t) bp->b_data + bp->b_bufsize, | ||||
(vm_offset_t) bp->b_data + newbsize); | (vm_offset_t) bp->b_data + newbsize); | ||||
if (origbuf != NULL) { | if (origbuf != NULL) { | ||||
bcopy(origbuf, bp->b_data, origbufsize); | bcopy(origbuf, bp->b_data, origbufsize); | ||||
free(origbuf, M_BIOBUF); | free(origbuf, M_BIOBUF); | ||||
} | } | ||||
bufspace_adjust(bp, newbsize); | bufspace_adjust(bp, newbsize); | ||||
Show All 23 Lines | allocbuf(struct buf *bp, int size) | ||||
if (bp->b_bcount == size) | if (bp->b_bcount == size) | ||||
return (1); | return (1); | ||||
if (bp->b_kvasize != 0 && bp->b_kvasize < size) | if (bp->b_kvasize != 0 && bp->b_kvasize < size) | ||||
panic("allocbuf: buffer too small"); | panic("allocbuf: buffer too small"); | ||||
newbsize = roundup2(size, DEV_BSIZE); | newbsize = roundup2(size, DEV_BSIZE); | ||||
if ((bp->b_flags & B_VMIO) == 0) { | if ((bp->b_flags & B_VMIO) == 0) { | ||||
if ((bp->b_flags & B_MALLOC) == 0) | |||||
newbsize = round_page(newbsize); | |||||
/* | /* | ||||
Not Done Inline ActionsI do not see why would check for bufmallocspace needed there. kib: I do not see why would check for bufmallocspace needed there. | |||||
Not Done Inline ActionsI suppose it's not. It just seemed clearer from a readability standpoint to call want_malloc_buf() here. If you feel strongly about it I can remove the function and just inline the if checks in both places where they're needed. darrick.freebsd_gmail.com: I suppose it's not. It just seemed clearer from a readability standpoint to call… | |||||
Not Done Inline ActionsI suggest to remove the space check from want_malloc_buf() and keep the check at the line 4231 inlined. kib: I suggest to remove the space check from want_malloc_buf() and keep the check at the line 4231… | |||||
* Just get anonymous memory from the kernel. Don't | * Just get anonymous memory from the kernel. Don't | ||||
* mess with B_CACHE. | * mess with B_CACHE. | ||||
*/ | */ | ||||
if (newbsize < bp->b_bufsize) | if (newbsize < bp->b_bufsize) | ||||
vfs_nonvmio_truncate(bp, newbsize); | vfs_nonvmio_truncate(bp, newbsize); | ||||
else if (newbsize > bp->b_bufsize) | else if (newbsize > bp->b_bufsize) | ||||
vfs_nonvmio_extend(bp, newbsize); | vfs_nonvmio_extend(bp, newbsize); | ||||
} else { | } else { | ||||
▲ Show 20 Lines • Show All 1,225 Lines • Show Last 20 Lines |
Ah, by "inline," I had in mind literally moving the code inside the caller.
(The "inline" annotation is usually discouraged since modern (C99+) compilers are free to more or less ignore it and use their own heuristics anyway.)