On Thu, Oct 24, 2019 at 10:55:06AM -0700, Bart Van Assche wrote:
> On 10/24/19 10:42 AM, Omar Sandoval wrote:
> > On Mon, Oct 21, 2019 at 03:57:19PM -0700, Bart Van Assche wrote:
> > > +# Configure one null_blk instance.
> > > +configure_null_blk() {
> > > + (
> > > + cd /sys/kernel/config/nullb || return $?
> > > + (
> > > + mkdir -p nullb0 &&
> > > + cd nullb0 &&
> > > + echo 0 > completion_nsec &&
> > > + echo 512 > blocksize &&
> > > + echo 16 > size &&
> > > + echo 1 > memory_backed &&
> > > + echo 1 > power
> > > + )
> > > + ) &&
> > > + ls -l /dev/nullb* &>>"$FULL"
> >
> > What's the point of these nested subshells? Can't this just be:
> >
> > configure_null_blk() {
> > cd /sys/kernel/config/nullb &&
> > mkdir -p nullb0 &&
> > cd nullb0 &&
> > echo 0 > completion_nsec &&
> > echo 512 > blocksize &&
> > echo 16 > size &&
> > echo 1 > memory_backed &&
> > echo 1 > power &&
> > ls -l /dev/nullb* &>>"$FULL"
> > }
>
> The above code is not equivalent to the original code because it does not
> restore the original working directory.
>
> When using 'cd' inside a subshell, the working directory that was effective
> before the 'cd' command is restored automatically when exiting from the
> subshell. I prefer using 'cd' in a subshell instead of using pushd / popd
> because with the former approach the old working directory is restored no
> matter how the exit from the subshell happens.
Ah, right, I didn't pay enough attention to the cd. In that case, I'd
prefer not doing the cd at all. Something like:
configure_null_blk() {
local nullb0="/sys/kernel/config/nullb/nullb0"
mkdir -p "$nullb0" &&
echo 0 > "$nullb0/completion_nsec" &&
echo 512 > "$nullb0/blocksize" &&
echo 16 > "$nullb0/size" &&
echo 1 > "$nullb0/memory_backed" &&
echo 1 > "$nullb0/power" &&
ls -l /dev/nullb* &>>"$FULL"
}
> > > +modify_nr_hw_queues() {
> > > + local deadline num_cpus
> > > +
> > > + deadline=$(($(_uptime_s) + TIMEOUT))
> > > + num_cpus=$(find /sys/devices/system/cpu -maxdepth 1 -name 'cpu[0-9]*' |
> > > + wc -l)
> >
> > Please just use nproc. Or even better, can you just read the original
> > value of /sys/kernel/config/nullb/nullb0/submit_queues? Or does that
> > start at 1?
>
> I will have a closer look and see which alternative works best.
Thanks!
