Re: IFEF_MPSAFE
On Wed, Nov 15, 2017 at 1:53 PM, Ryota Ozaki wrote: > On Fri, Nov 10, 2017 at 6:35 PM, Ryota Ozaki wrote: >> Hi, >> >> http://www.netbsd.org/~ozaki-r/IFEF_MPSAFE.diff >> >> I'm going to commit the above change that integrates >> IFEF_OUTPUT_MPSAFE and IFEF_START_MPSAFE flags into >> IFEF_MPSAFE. >> >> The motivation is to not waste if_extflags bits. I'm now >> trying to make if_ioctl() hold KERNEL_LOCK selectively >> for some reasons as well as if_start() and if_output(). > > BTW this is a patch for this plan: > http://www.netbsd.org/~ozaki-r/if_ioctl-no-KERNEL_LOCK.diff > > It removes KERNEL_LOCK for if_ioctl from soo_ioctl and > selectively takes it in doifioctl. To this end, some fine-grain > KERNEL_LOCKs have to be added where calling components/functions > that aren't MP-safe. Revised rebased on latest NET_MPSAFE macros. The new patch also provides similar macros: http://www.netbsd.org/~ozaki-r/if_ioctl-no-KERNEL_LOCK.revised.diff ozaki-r
Re: ext2fs superblock updates
On Thu, Nov 16, 2017 at 12:12 PM, Mouse wrote: > >> They are generated by _newfs_ and left untouched thereafter. > > Interesting, thanks. what's so useful about the superblock at newfs > > time? > > It contains enough information for fsck to find other critical things > (like cylinder groups and their inode tables). If the primary > superblock has been destroyed but the rest of the filesystem is intact, > fsck is supposed be able to put the filesystem back together with the > help of a backup superblock. Yes. For UFS filesystems on BSD labeled disks, there's additional hints to fsck about the size of different parts of the filesystem that allow it to guess fairly well at the location of these alternate super blocks. Warner
Re: increase softint_bytes
softint_establish() already fails if there is no space, and prints a
WARNING in this case. The caller however needs to check for the failure,
which ixgbe(4) mostly doesn't. Not sure if that is really the root cause,
the panic seems to be actually in different area, and I didn't actually see
the warning in the dmesg.
Making softint size fully dynamic is unfortunately not straighforward, it
would need to reallocate the softint memory on all CPUs in a way that any
serviced softints would continue working during this time. It would be
simple however to adjust it based on % of physical memory, instead of fixed
value.
If I count correctly, with current 8192 bytes the system supports some 100
softints, which seems to be on quite low side - more advanced hardware and
drivers usually use queue and softint per cpu, so it can quickly run out on
system with e.g. 32 cores.
I agree that the sysctl seems somewhat unnecessary, since the limit is only
relevant during boot anyway, so it's not very helpful.
Jaromir
2017-11-16 21:05 GMT+01:00 matthew green :
> Masanobu SAITOH writes:
> > Hi, all.
> >
> > Some device drivers now allocate a lot of softints.
> > See:
> >
> > http://mail-index.netbsd.org/current-users/2017/11/09/
> msg032581.html
> >
> > To avoid this panic, I wrote the following patch:
> >
> > http://www.netbsd.org/~msaitoh/softint-20171116-0.dif
> >
> > Summary:
> >
> > - Increase the default size from 8192 bytes to 32768 bytes.
> > - Add new option SOFTINT_BYTES to change the value.
> > - Add two new read-only sysctls kern.softint.{max,count}
> >
> > Any comment?
>
> can't this be fixed by making it dynamic? ie, fix it properly so
> that it never has this problem again -- if in 10 years we're adding
> 5x as many softints as today, we'll hit it again.
>
> i also don't see why the crash can't be fixed to become an error.
> the code tries to avoid adding more than it can, but soemthing must
> be wrong for the crash to occur anyway. ie, at worst we'd fix it
> to return an error instead of crashing.
>
> if that is done, the sysctl exports seem unneccesary as well.
>
> thanks.
>
>
> .mrg.
>
Re: increase softint_bytes
On 2017/11/17 6:25, Jaromír Doleček wrote:
softint_establish() already fails if there is no space, and prints a WARNING in
this case. The caller however needs to check for the failure, which ixgbe(4)
mostly doesn't.
No. ixgbe() checks the return value of if_initialize() and return
correctly. That panic is not in ixgbe(4) driver but in ip_input.c::ip_init().
Not sure if that is really the root cause, the panic seems to be actually in
different area, and I didn't actually see the warning in the dmesg.
Making softint size fully dynamic is unfortunately not straighforward, it would
need to reallocate the softint memory on all CPUs in a way that any serviced
softints would continue working during this time.
Each driver which call softint_establish() keep the return value, so
it's very hard without big modification.
It would be simple however to adjust it based on % of physical memory, instead
of fixed value.
It might be good, but it's only small area, so fixed value might be OK.
And not so many people create a lot of pseudo interface, so I think it's
OK to make the max value as a kernel option to increase.
If I count correctly, with current 8192 bytes the system supports some 100
softints, which seems to be on quite low side - more advanced hardware and
drivers usually use queue and softint per cpu, so it can quickly run out on
system with e.g. 32 cores.
I agree that the sysctl seems somewhat unnecessary, since the limit is only
relevant during boot anyway,
As I wrote, it's incorrect because ifconfig xxxN create allocates softint.
so it's not very helpful.
Jaromir
2017-11-16 21:05 GMT+01:00 matthew green mailto:m...@eterna.com.au>>:
Masanobu SAITOH writes:
> Hi, all.
>
> Some device drivers now allocate a lot of softints.
> See:
>
> http://mail-index.netbsd.org/current-users/2017/11/09/msg032581.html
<http://mail-index.netbsd.org/current-users/2017/11/09/msg032581.html>
>
> To avoid this panic, I wrote the following patch:
>
> http://www.netbsd.org/~msaitoh/softint-20171116-0.dif
<http://www.netbsd.org/~msaitoh/softint-20171116-0.dif>
>
> Summary:
>
> - Increase the default size from 8192 bytes to 32768 bytes.
> - Add new option SOFTINT_BYTES to change the value.
> - Add two new read-only sysctls kern.softint.{max,count}
>
> Any comment?
can't this be fixed by making it dynamic? ie, fix it properly so
that it never has this problem again -- if in 10 years we're adding
5x as many softints as today, we'll hit it again.
i also don't see why the crash can't be fixed to become an error.
the code tries to avoid adding more than it can, but soemthing must
be wrong for the crash to occur anyway. ie, at worst we'd fix it
to return an error instead of crashing.
if that is done, the sysctl exports seem unneccesary as well.
thanks.
.mrg.
--
---
SAITOH Masanobu (msai...@execsw.org
msai...@netbsd.org)
Re: increase softint_bytes
Hi, mrg.
On 2017/11/17 5:05, matthew green wrote:
Masanobu SAITOH writes:
Hi, all.
Some device drivers now allocate a lot of softints.
See:
http://mail-index.netbsd.org/current-users/2017/11/09/msg032581.html
To avoid this panic, I wrote the following patch:
http://www.netbsd.org/~msaitoh/softint-20171116-0.dif
Summary:
- Increase the default size from 8192 bytes to 32768 bytes.
- Add new option SOFTINT_BYTES to change the value.
- Add two new read-only sysctls kern.softint.{max,count}
Any comment?
can't this be fixed by making it dynamic?
It's not easy because the return value of softint_establish() is
made from this area's address. As you know, the value is keep by each driver.
ie, fix it properly so
that it never has this problem again -- if in 10 years we're adding
5x as many softints as today, we'll hit it again.
i also don't see why the crash can't be fixed to become an error.
The code is:
ip_init(void)
{
const struct protosw *pr;
in_init();
sysctl_net_inet_ip_setup(NULL);
pr = pffindproto(PF_INET, IPPROTO_RAW, SOCK_RAW);
KASSERT(pr != NULL);
ip_pktq = pktq_create(IFQ_MAXLEN, ipintr, NULL);
KASSERT(ip_pktq != NULL); <== This.
the code tries to avoid adding more than it can, but soemthing must
be wrong for the crash to occur anyway.
I think this KASSERT() is reasonable.
ie, at worst we'd fix it
to return an error instead of crashing.
if that is done, the sysctl exports seem unneccesary as well.
bridge, agr, strip, sl, stf, ppp, l2tp, gre and maybe some other
pseudo interfaces which is created by ifconfig command allocate
softint. Some of them may be created tens, hundreds or more.
So I think it's worth to check the current number.
thanks.
.mrg.
--
---
SAITOH Masanobu (msai...@execsw.org
msai...@netbsd.org)
re: increase softint_bytes
Masanobu SAITOH writes:
> Hi, all.
>
> Some device drivers now allocate a lot of softints.
> See:
>
> http://mail-index.netbsd.org/current-users/2017/11/09/msg032581.html
>
> To avoid this panic, I wrote the following patch:
>
> http://www.netbsd.org/~msaitoh/softint-20171116-0.dif
>
> Summary:
>
> - Increase the default size from 8192 bytes to 32768 bytes.
> - Add new option SOFTINT_BYTES to change the value.
> - Add two new read-only sysctls kern.softint.{max,count}
>
> Any comment?
can't this be fixed by making it dynamic? ie, fix it properly so
that it never has this problem again -- if in 10 years we're adding
5x as many softints as today, we'll hit it again.
i also don't see why the crash can't be fixed to become an error.
the code tries to avoid adding more than it can, but soemthing must
be wrong for the crash to occur anyway. ie, at worst we'd fix it
to return an error instead of crashing.
if that is done, the sysctl exports seem unneccesary as well.
thanks.
.mrg.
Re: ext2fs superblock updates
>> They are generated by _newfs_ and left untouched thereafter. > Interesting, thanks. what's so useful about the superblock at newfs > time? It contains enough information for fsck to find other critical things (like cylinder groups and their inode tables). If the primary superblock has been destroyed but the rest of the filesystem is intact, fsck is supposed be able to put the filesystem back together with the help of a backup superblock. > is that for disaster recovery when half my drive is gone and I might > be able to salvage something from a cylinder? Yes...or something has trashed the primary superblock but not (most of) the rest of the filesystem. For example, if you start copying raw disks and carelessly write to the wrong device, and stop it early, you might be able to salvage most of the damaged filesystem that way. In practice, I would say they are largely historical artifacts at this point - I can't remember the last time a backup superblock was of use to me. (It's happened, maybe as many as five times, but the most recent was I-can't-recall-when ago.) They made a lot more sense in the days before disk drives that auto-spare flaky sectors (the superblock gets ovewritten heavily) and sysadmins a significant fraction of whom were competent to do things like manually repair damaged filesystems. /~\ The ASCII Mouse \ / Ribbon Campaign X Against HTMLmo...@rodents-montreal.org / \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B
Re: ext2fs superblock updates
On Thu, Nov 16, 2017 at 09:54:44AM -0500, Mouse wrote: > They are generated by _newfs_ and left untouched thereafter. Interesting, thanks. what's so useful about the superblock at newfs time? is that for disaster recovery when half my drive is gone and I might be able to salvage something from a cylinder?
Re: ext2fs superblock updates
>> From the looks of it, the ext2fs code only updates the primary
>> superblock: [...]
> [...similarity to FFS...]
I see two mistakes. ("I will proofread my list posts. I will
proofread my list posts. I will proofread my list posts. ...") I was
going to ignore the first one, but the other one is actually somewhat
important.
> I don't know ext2. But I have seen it said that it's basically slimiar
s/slimiar/similar/
>[...] Thus, there is never any need to update the
> backup superblocks - they are generated by fsck and left untouched
> thereafter.
They are generated by _newfs_ and left untouched thereafter.
/~\ The ASCII Mouse
\ / Ribbon Campaign
X Against HTMLmo...@rodents-montreal.org
/ \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B
Re: ext2fs superblock updates
> From the looks of it, the ext2fs code only updates the primary > superblock: [...] > I'm under the impression that the secondary ones exist as backups and > are meant to be usable. > Is this wrong or am I misunderstanding things? I don't know ext2. But I have seen it said that it's basically slimiar to FFS, and I do know FFS. If we were talking about FFS instead, I'd say, the backup superblocks are meant to be usable - for fsck. Not for live use. In particular, all the superblock data that FFS mutates in live use is data that fsck knows how to regenerate. Thus, there is never any need to update the backup superblocks - they are generated by fsck and left untouched thereafter. How applicable those remarks are to ext2 depends, of course, on how similar ext2 is to FFS in these respects. /~\ The ASCII Mouse \ / Ribbon Campaign X Against HTMLmo...@rodents-montreal.org / \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B
Re: ext2fs superblock updates
Hi. > On 16 Nov 2017, at 10:40, co...@sdf.org wrote: > > From the looks of it, the ext2fs code only updates the primary > superblock: > (…) > I'm under the impression that the secondary ones exist as > backups and are meant to be usable. > > Is this wrong or am I misunderstanding things? As far as I know, Linux does not update backup superblocks under normal operation. Only primary is updated. Backup superblock are always set to “not clean”, so that free blocks/inodes/last mount are expected to be bad there. So this behaviour of ext2fs_sbupdate is most likely correct. Best regards, Radoslaw signature.asc Description: Message signed with OpenPGP
ext2fs superblock updates
Hi folks.
>From the looks of it, the ext2fs code only updates the primary
superblock:
/*
* Write a superblock and associated information back to disk.
*/
int
ext2fs_sbupdate(struct ufsmount *mp, int waitfor)
{
struct m_ext2fs *fs = mp->um_e2fs;
struct buf *bp;
int error = 0;
bp = getblk(mp->um_devvp, SBLOCK, SBSIZE, 0, 0);
e2fs_sbsave(&fs->e2fs, (struct ext2fs*)bp->b_data);
if (waitfor == MNT_WAIT)
error = bwrite(bp);
else
bawrite(bp);
return error;
}
I'm under the impression that the secondary ones exist as
backups and are meant to be usable.
Is this wrong or am I misunderstanding things?
Thanks.
increase softint_bytes
Hi, all.
Some device drivers now allocate a lot of softints.
See:
http://mail-index.netbsd.org/current-users/2017/11/09/msg032581.html
To avoid this panic, I wrote the following patch:
http://www.netbsd.org/~msaitoh/softint-20171116-0.dif
Summary:
- Increase the default size from 8192 bytes to 32768 bytes.
- Add new option SOFTINT_BYTES to change the value.
- Add two new read-only sysctls kern.softint.{max,count}
Any comment?
--
---
SAITOH Masanobu (msai...@execsw.org
msai...@netbsd.org)
