User Details
- User Since
- Jun 3 2014, 6:27 PM (469 w, 16 h)
Mon, May 29
Mon, May 22
Mon, May 15
Sat, May 6
Tue, May 2
Apr 27 2023
Apr 18 2023
I have no objections in general, if you find it useful, just a few on the implementation.
Apr 17 2023
Apr 16 2023
I also wonder if we should make the same tunable to affect other forms of sleep, or not. Is this the most pathological in implementations? May be so. considering there were no complains about others.
Apr 14 2023
The additional flags are normally used only for queued requests. So some of places here are likely not affected. But it should not harm probably any way.
Mar 27 2023
Mar 18 2023
The original physical path format came from Solaris/Illumos. While I agree that it is ugly and not very documented, I am not a big fan of incompatible changes. I'd think twice.
Mar 17 2023
ses_process_control_request() triggered by ses_set_enc_status() already calls ses_poll_status(), so most of what you are doing should already work. Not sure only about ENCIOC_SETSTRING.
Mar 16 2023
Mar 14 2023
Mar 13 2023
It may be OK, but I am not sure I like this approach. It creates additional kernel requests, it is potentially racy. Doesn't the enclosure return some reasonable status that kernel could convert into EACCESS or something more reasonable than EINVAL? At least it would be good to do both.
Mar 9 2023
Mar 8 2023
Feb 15 2023
Feb 13 2023
I have no objections, just some comments to polish.
Jan 31 2023
From perspective of ZFS I am all for the removal of mmap() use. ZFS does not benefit from it, but only suffers badly from created buffer cache pressure. We have it disabled on TrueNAS. I was tempted to remove it, but didn't want to open large discussion. I guess it may be beneficial for UFS and any FS using buffer cache, avoiding one memory copy. But I think it should be handled by copy_file_range() inside the kernel in a way the most efficient for specific file system(s).
Jan 27 2023
Jan 21 2023
Jan 20 2023
I suppose that request would give you all traffic of the LUNs accessible through that target, even one going though different targets. As alternative, CTL collects per-port statistics, that would closer match targets, if desired, but then without separation between different LUNs.
I've never before looked on this code, but all this targdata.targets[lun] logic looks wrong to me. Any LUN may be mapped into multiple different ports, think that this code does not allow. LUNs not mapped into any port is only a one minor case of generally broken design. I have no idea what data this Prometheus is expecting to receive, but right now it obviously does not receive the full picture. This particular patch is not bad, but it just does not fix whole problem, only a minor part.
Jan 10 2023
Dec 17 2022
Dec 13 2022
Dec 9 2022
For SMP the value that holds the next hardclock event is per-CPU, and what prevents those per-CPU values from sliding between CPUs? During boot, the dummy timer will put "random" starting values in there? Or am I wrong?
Dec 8 2022
Dec 6 2022
I've never used libxo to closely review this, but I find it amusing that kernel produces XML for libgeom to parse for geom tool to be able to produce another XML (or JSON or whatever). :)
I was thinking about removing 64bit divisions in the hot path and moving them into the first initialization, instead you've added more... We should not bother about alignment in callout_when() if hardclocktime is aligned. Time passed by caller must be aligned to tick_sbt already by him, otherwise he should not use C_HARDCLOCK.
Dec 3 2022
Why do you think this is needed? All C_HARDCLOCK consumers are supposed to pass only periods multiple of tick_sbt, and hardclocktime above should be aligned to hardclock also. In what cases do you see misaligned values?
Nov 28 2022
Looks good to me, but I am not sure it makes sense to still have the paragraph about 11.0. 11 branch is out of support now, plus that -T option of geli seems to disable TRIM, not enable it, that would have some more sense to document.
Nov 8 2022
Not that I have specific objections against this, but I think it would be much better to implement a generic way to print all consumers connected to specified GEOM provider. This code does not look anyhow specific to gmultipath.
Nov 5 2022
On TrueNAS we do not upgrade pools automatically to allow users to make sure system operates properly to keep possibility of downgrade if anything go wrong. But if this works only on first boot after install it should probably be fine.
Oct 27 2022
Oct 23 2022
Oct 22 2022
I agree with Konstatin's comments, otherwise looks good to me. Thinking about the HZ_MAXIMUM value I wonder where is " >> 6" coming from? Is it useful to allow hz > 100KHz? But then above ~15KHz " >> 6" will loose precision. If HZ_MAXIMUM is set to ~68K, then " >> 5" could be better here.
Oct 21 2022
I always wondered whether kernel could try harder to reduce the number of segments below the number of pages to help hardware that can benefit from it. But you are right that current kernel does not even try to allocate the bounce pages consequently, so allocating more pages than segments supported does not make much sense. Though many drivers should already request maximum map size as maximum number of segments multiplied by page size. So this should affect only drivers that are not doing it.
Oct 20 2022
Oct 19 2022
Oct 18 2022
Oct 11 2022
Oct 10 2022
I think this may create a race between new command completion added into the queue and already running handler, when interrupt arriving last moment get dropped, but newly completed command is not yet processed.
Since it is under the queue lock I don't see much difference in the order, only removal of the condition makes sense to me.
Oct 8 2022
Oct 6 2022
Oct 4 2022
Oct 3 2022
Oct 2 2022
Oct 1 2022
Sep 30 2022
The original idea was to keep the potentially expensive clock reading operation out of the lock scope, which in case of global timer, like HPET (which I guess you are using according to other review), may be quite congested. You are right that reading time counter closer to event timer programming should reduce the jitter. I just have difficulties to believe that spin on the lock may be long enough to be possible to hear (thinking whether there can be a context switch somewhere?). But if you say so, I have no big objections. I don't know any modern system with global event timer or slow enough time counter any more, so don't care so much about this aspect.
et_start() should guarantee that this code receive first and/or period big enough for meaningful fdiv. Do you expect this code to loop enough times for overflow the fdiv turning it zero?
Sep 27 2022
Sep 16 2022
Sep 14 2022
Sep 13 2022
Looks good to me, though I am not really qualified to choose. I just saw CUBIC recovering much faster after packet losses in datacenter environments without flow control and WAN/WiFi. With NewReno we saw regular pauses in a traffic up to a second IIRC.