Page MenuHomeFreeBSD

nikita_elyzion.net (Nikita Kozlov)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 2 2016, 7:55 PM (181 w, 12 h)

Recent Activity

Dec 13 2017

nikita_elyzion.net added a comment to D13447: zfs: deadlock in "zfs rename" if zv_total_opens > 0 since MFV r323535.

I forgot to add, even with D7179, it seems still be a good idea to ensure that we are not trying to rename a zvol which is still opened somewhere.

Dec 13 2017, 7:56 PM
nikita_elyzion.net added a comment to D13447: zfs: deadlock in "zfs rename" if zv_total_opens > 0 since MFV r323535.

I was not aware of D7179, thanks to pointing it. I tried it in some simple case and it seems to work pretty well.
Moving zfsvfs_update_fromname() and zvol_rename_minors() from txg_sync to the ioctl part was something I considered but I was unsure of the result because of the part of the rename still in txg_sync but after a second look it seems fine.
About the race, maybe adding a way to lock a specific zvol by adding something in zvol_state (from zvol.c) and holding the lock on the specific zvol during a zfs_ioc_rename could be enough to fix the issue ? so we won't have concurrent rename on the same zvol ? or am I missing another race here ?
My feeling is that we have now a second obvious deadlock on zfs rename in stable/11 and it would be better to use your patch now with (or maybe even without) a lock on the zvol than what we have now.

Dec 13 2017, 7:52 PM

Dec 11 2017

nikita_elyzion.net created D13447: zfs: deadlock in "zfs rename" if zv_total_opens > 0 since MFV r323535.
Dec 11 2017, 2:44 PM

Nov 20 2017

nikita_elyzion.net added a comment to D13088: zpool: cannot anymore import cache device which doesn't contain a previos ZFS vdev label.
if (zpool_read_label(fd, &config) != 0 && errno == ENOMEM) {
        (void) no_memory(hdl);
        return (-1);
}

would be:

if (zpool_read_label(fd, &config) == -1) {
        (void) no_memory(hdl);
        return (-1);
}

Maybe instead:

if (zpool_read_label(fd, &config) != 0 && errno == ENOMEM) {
        (void) no_memory(hdl);
        /* After calling no_memory(), we can reset errno */
        errno = 0;
        return (-1);
}
Nov 20 2017, 4:29 AM

Nov 17 2017

nikita_elyzion.net added a comment to D13088: zpool: cannot anymore import cache device which doesn't contain a previos ZFS vdev label.

Thanks for your comments Allan and Andriy.
Concerning the errno set to 12 (ENOMEM), it's effectively set by the ioctl ZFS_IOC_POOL_STATS which is first called with a too small nvlist.
I'm attaching the conditional watchpoint where it happens.

Nov 17 2017, 3:45 PM

Nov 15 2017

nikita_elyzion.net added a comment to D13093: cam/scsi_da: active ssd quirks for all SAMSUNG MZ7* ssd.
In D13093#272417, @imp wrote:

Now that you point it, yes I missed that part!
It will be difficult for me to test it, do you want that I path ata_da.c anyway ?

The 4k quirk is in serious need of being revisited because we shouldn't have to do this except for a tiny tiny fraction of drives.

Nov 15 2017, 3:44 PM
nikita_elyzion.net updated the diff for D13093: cam/scsi_da: active ssd quirks for all SAMSUNG MZ7* ssd.
Nov 15 2017, 3:40 PM

Nov 14 2017

nikita_elyzion.net added a comment to D13093: cam/scsi_da: active ssd quirks for all SAMSUNG MZ7* ssd.

Now that you point it, yes I missed that part!
It will be difficult for me to test it, do you want that I path ata_da.c anyway ?

Nov 14 2017, 8:10 PM
nikita_elyzion.net created D13093: cam/scsi_da: active ssd quirks for all SAMSUNG MZ7* ssd.
Nov 14 2017, 7:40 PM
nikita_elyzion.net added a reviewer for D13088: zpool: cannot anymore import cache device which doesn't contain a previos ZFS vdev label: bapt.
Nov 14 2017, 7:03 PM
nikita_elyzion.net created D13088: zpool: cannot anymore import cache device which doesn't contain a previos ZFS vdev label.
Nov 14 2017, 4:12 PM

Oct 14 2017

nikita_elyzion.net updated the diff for D12646: usr.sbin/ctld: fix a segfault on duplicate lun or target when using ucl conf format.
Oct 14 2017, 12:03 AM

Oct 12 2017

nikita_elyzion.net created D12646: usr.sbin/ctld: fix a segfault on duplicate lun or target when using ucl conf format.
Oct 12 2017, 4:02 PM

Sep 7 2017

nikita_elyzion.net updated the diff for D12163: [RFC]: decrement needfree by the amount of evicted cache.

I took some time to dig more in the code of arc_adjust()/arc_shrink()/arc_kmem_reap_now() and the pagedaemon/vmmeter and the suggestion of avg seems good.
freemem is actually #define freemem vm_cnt.v_free_count (maybe we can use another stats from vmmeter/pagedaemon to replace the needfree by another define ?)
I have also improved my test to reproduce more easily the issue and removing the needfree variable seems good.

Sep 7 2017, 10:26 PM

Sep 6 2017

nikita_elyzion.net added inline comments to D12181: [RFC] limit the max number of iscsi connections to a specific target.
Sep 6 2017, 12:47 PM
nikita_elyzion.net added a comment to D12181: [RFC] limit the max number of iscsi connections to a specific target.

I like the idea - this would allow us to say "no more than one initiator at a time", greatly simplifying host standby (cluster) setups.
That said, from my understanding there's a race condition - if two initiators try to connect at the same time, they will both make it through the login phase, since each ctld(8) child doesn't know about the other. I believe the check should happen in the kernel, during handoff. The iSCSI spec already contains a mechanism for the target to request logout (or just drop the connection) using the Asynchronous Message PDU; I'd expect initiators to be able to handle it gracefully, as it's mandatory part of the spec - although I've never tried that myself.
I'd also suggest a different name; other ctl.conf parameters don't use underscores, and mostly avoid contractions. How about "connections-max"?
And yeah, the man page :-)

Sep 6 2017, 12:37 PM

Aug 31 2017

nikita_elyzion.net added inline comments to D12181: [RFC] limit the max number of iscsi connections to a specific target.
Aug 31 2017, 3:51 PM
nikita_elyzion.net updated the diff for D12181: [RFC] limit the max number of iscsi connections to a specific target.

Updating the diff, I forgot to push a typo fix from a repo.

Aug 31 2017, 3:14 PM
nikita_elyzion.net created D12181: [RFC] limit the max number of iscsi connections to a specific target.
Aug 31 2017, 3:08 PM
nikita_elyzion.net added a comment to D12163: [RFC]: decrement needfree by the amount of evicted cache.
In D12163#252767, @avg wrote:

Can you try to experiment with removing needfree altogether?
I mean both the code that sets it (in arc_lowmwem and in arc_reclaim_thread) and the code that uses it (in arc_available_memory and in arc_reclaim_thread).
FreeBSD needfree does not appear to be very useful.

Aug 31 2017, 2:17 PM

Aug 30 2017

nikita_elyzion.net created D12163: [RFC]: decrement needfree by the amount of evicted cache.
Aug 30 2017, 10:42 AM

Aug 29 2017

nikita_elyzion.net added a comment to D12144: add sysctls for arc shrinking and growing values.
In D12144#252284, @manu wrote:
In D12144#252281, @bapt wrote:

Looks good to me, but I will wait for people with more internal knowledge than I have about ZFS to step in anyway

Same for me.
Nikita: Could you provide numbers ? (i.e.: when does one need to change the default value)

Aug 29 2017, 6:15 PM
nikita_elyzion.net created D12151: usr.bin/netstat: don't call kresolve_list() if using live kernel.
Aug 29 2017, 1:42 PM

Aug 28 2017

nikita_elyzion.net added a comment to D12144: add sysctls for arc shrinking and growing values.

However you generated this DR, Phabricator didn't get enough context info. The best way to start a code review is to use php5-arcanist according to the instructions at https://wiki.freebsd.org/Phabricator . But if you must use the web interface, remember to use "-D99999" when creating the diff to ensure that maximal context is available.

Aug 28 2017, 3:08 PM
nikita_elyzion.net updated the diff for D12144: add sysctls for arc shrinking and growing values.
Aug 28 2017, 3:05 PM
nikita_elyzion.net created D12144: add sysctls for arc shrinking and growing values.
Aug 28 2017, 1:03 PM

Jun 27 2017

nikita_elyzion.net updated the diff for D11372: usr.sbin/sesutil: Introduce libxo support to sesutil(8).

The 'q' in {q:led/locate} is necessary for the json output at least, without that we have : "led":locate instead of "led":"locate"

Jun 27 2017, 10:11 PM
nikita_elyzion.net updated the diff for D11372: usr.sbin/sesutil: Introduce libxo support to sesutil(8).

Fixed bapt and allanjude comments.
I'm keeping .Dd as is because bapt seems to prefer to update it at commit time.

Jun 27 2017, 10:08 PM

Jun 26 2017

nikita_elyzion.net created D11372: usr.sbin/sesutil: Introduce libxo support to sesutil(8).
Jun 26 2017, 8:32 PM

Jun 23 2017

nikita_elyzion.net accepted D11324: Refactor print the extrat status in sesutil.
Jun 23 2017, 3:05 PM

Aug 2 2016

nikita_elyzion.net updated the diff for D7399: truss: fix uninitialized trussinfo->curthread in add_threads()/enter_syscall.

updating with jhb comments

Aug 2 2016, 9:32 PM
nikita_elyzion.net retitled D7399: truss: fix uninitialized trussinfo->curthread in add_threads()/enter_syscall from to truss: fix uninitialized trussinfo->curthread in add_threads()/enter_syscall.
Aug 2 2016, 8:19 PM