Commit Graph

2155 Commits

Author SHA1 Message Date
pfg
ced875130d Revert r327828, r327949, r327953, r328016-r328026, r328041:
Uses of mallocarray(9).

The use of mallocarray(9) has rocketed the required swap to build FreeBSD.
This is likely caused by the allocation size attributes which put extra pressure
on the compiler.

Given that most of these checks are superfluous we have to choose better
where to use mallocarray(9). We still have more uses of mallocarray(9) but
hopefully this is enough to bring swap usage to a reasonable level.

Reported by:	wosch
PR:		225197
2018-01-21 15:42:36 +00:00
asomers
2efe3d3999 gnop(8): add the ability to set a nop provider's physical path
While I'm here, expand the existing tests a bit.

MFC after:	3 weeks
Differential Revision:	https://reviews.freebsd.org/D13579
2018-01-18 05:57:10 +00:00
pfg
067d5edba6 misc geom and gnu: make some use of mallocarray(9).
Focus on code where we are doing multiplications within malloc(9). None of
these ire likely to overflow, however the change is still useful as some
static checkers can benefit from the allocation attributes we use for
mallocarray.

This initial sweep only covers malloc(9) calls with M_NOWAIT. No good
reason but I started doing the changes before r327796 and at that time it
was convenient to make sure the sorrounding code could handle NULL values.

Differential revision: https://reviews.freebsd.org/D13837
2018-01-15 21:23:16 +00:00
avg
e3bb7b0fbf geom_disk / scsi_da: deny opening write-protected disks for writing
Ths change consists of two parts.

geom_disk: deny opening a disk for writing if it's marked as
write-protected.  A new disk(9) flag is added to mark write protected
disks.  A possible alternative could be to add another parameter to d_open,
so that the open mode could be passed to it and the disk drivers could
make the decision internally, but the flag required less churn.

scsi_da: add a new phase of disk probing to query the all pages mode
sense page.  We can determine if the disk is write protected using bit 7
of the device specific field in the mode parameter header returned by
MODE SENSE.

PR:		224037
Reviewed by:	mav
MFC after:	4 weeks
Differential Revision: https://reviews.freebsd.org/D13360
2018-01-15 11:20:00 +00:00
markj
652ed11da6 Fix handling of read errors during mirror synchronization.
We would previously just free the request BIO, which would either cause
the disk to stay stuck in the SYNCHRONIZING state, or result in
synchronization completing without having copied the block which
returned an error.

With this change, if the disk which returned an error is the only active
disk in the mirror, the synchronizing disk is kicked out. Otherwise, the
read is retried.

Reported and tested by:	pho (previous version)
MFC after:	2 weeks
Sponsored by:	Dell EMC Isilon
2018-01-10 19:37:21 +00:00
markj
a2d6c6c5ac Clarify the use of the gmirror flag mask constants.
MFC after:	1 week
Sponsored by:	Dell EMC Isilon
2018-01-10 15:21:36 +00:00
markj
fa95b8c054 Avoid referencing a possibly freed consumer after r327496.
g_mirror_regular_request() may free the gmirror consumer for a disk
if that disk is being disconnected, after which we must not dereference
the consumer pointer.

CID:		1384280
X-MFC with:	r327496
2018-01-10 05:06:21 +00:00
markj
ab53928d62 Sort and remove unneeded includes.
MFC after:	1 week
Sponsored by:	Dell EMC Isilon
2018-01-08 15:56:40 +00:00
markj
807855890b Release the queue lock before restarting the worker loop.
Reported and tested by:	pho
MFC after:	3 days
Sponsored by:	Dell EMC Isilon
2018-01-08 15:41:49 +00:00
markj
6266e5e4ec Fix some I/O ordering issues in gmirror.
- BIO_FLUSH requests were dispatched to the disks directly from
  g_mirror_start() rather than going through the mirror's I/O request
  queue, so they could have been reordered with preceding writes.
  Address this by processing such requests from the queue, avoiding
  direct dispatch.
- Handling for collisions with synchronization requests was too
  fine-grained and could cause reordering of writes. In particular,
  BIO_ORDERED was not being honoured. Address this by effectively
  freezing the request queue any time a collision with a synchronization
  request occurs. The queue is unfrozen once the collision with the
  first frozen request is over.
- The above-mentioned collision handling allowed reads to jump ahead
  of writes to the same offset. Address this by freezing all request
  types when a collision occurs, not just BIO_WRITEs and BIO_DELETEs.

Also add some more fail points for use in testing error handling.

Reviewed by:	imp
MFC after:	3 weeks
Sponsored by:	Dell EMC Isilon
Differential Revision:	https://reviews.freebsd.org/D13559
2018-01-02 18:11:54 +00:00
cperciva
95c6ba719c Instrument "boot holds" for the benefit of the TSLOG framework. These
are places where the "main thread" of the booting kernel (either the
thread which later becomes swapper or the thread which later becomes
init) has to stop and wait for action to take place in another thread
before continuing.

There are currently three such holds:
1. The intr_config_hooks SYSINIT waits for hooks registered via the
config_intrhook_establish function; this allows (typically) devices
which need interrupts enabled to complete their initialization to do
so before root is mounted.

2. The g_waitidle function waits for the GEOM event queue to be empty;
this ensures that all of the disks which have been attached have been
tasted before we attempt to mount root.

3. The vfs_mountroot_wait function (in addition to calling g_waitidle)
waits for holds registered via root_mount_hold; among other things, this
is used by the USB subsystem to ensure that we don't fail to mount root
if it's located on a USB disk which takes a while to probe.
2017-12-31 09:23:52 +00:00
pfg
6a3873d1b1 geom_ccd.c: Fix the licenses properly
The license merging in r109471 didn't take into account that licensing
could change. Just removing the 3rd clause obviates the copyright
assignment to the NetBSD Foundation.

We do have plenty of files that have two or more licensing as in this
case, so fix this properly by splitting back the licenses as they are
upstream.

Obtained from:	NetBSD
2017-12-30 02:07:18 +00:00
pfg
215db6c13d geom_ccd.c: Update the license with changes from upstream.
Part of this file originated in NetBSD, with the original file
carrying two versions of 4-clause BSD licenses. r109471 attempted to
simplify the situation by putting both licenses together.

Meanwhile, NetBSD dropped Clauses 3 and 4 from their own license, and
eventually NetBSD got permission from the University of Utah to drop the
3rd clause.

Keep the license "simple" by dropping the third clause since both TNF,
Utah/Berkeley and phk agree in principle that it can be dropped.

Obtained from:	NetBSD (ccd.c CVS 1.128, 1.138)
2017-12-30 01:37:08 +00:00
kan
c8da6fae2c Do pass removing some write-only variables from the kernel.
This reduces noise when kernel is compiled by newer GCC versions,
such as one used by external toolchain ports.

Reviewed by: kib, andrew(sys/arm and sys/arm64), emaste(partial), erj(partial)
Reviewed by: jhb (sys/dev/pci/* sys/kern/vfs_aio.c and sys/kern/kern_synch.c)
Differential Revision: https://reviews.freebsd.org/D10385
2017-12-25 04:48:39 +00:00
markj
6afde2097a Avoid using bioq_* in gmirror.
gmirror does not perform any sorting of I/O requests, so the bioq API
doesn't provide any advantages over plain TAILQs. The API also does not
provide operations needed by an upcoming change.

No functional change intended. The diff shrinks the geom_mirror.ko
text and the gmirror softc slightly.

Tested by:	pho (part of a larger patch)
MFC after:	1 week
Sponsored by:	Dell EMC Isilon
2017-12-19 17:13:04 +00:00
markj
76756e3bf3 Give a couple of predication functions a bool return type.
No functional change intended.

MFC after:	1 week
Sponsored by:	Dell EMC Isilon
2017-12-15 19:14:21 +00:00
markj
da31570b57 Typo.
MFC after:	1 week
2017-12-15 19:03:03 +00:00
markj
48eee215b9 Address a possible lost wakeup for gmirror events.
g_mirror_event_send() acquires the I/O queue lock to deliver a wakeup
to the worker thread, and this is done after enqueuing the event.
So it's sufficient to check the event queue before atomically releasing
the queue lock and going to sleep.

MFC after:	1 week
Sponsored by:	Dell EMC Isilon
2017-12-12 17:29:34 +00:00
markj
1f07d15939 Give g_mirror_event_get() a more accurate name.
MFC after:	1 week
Sponsored by:	Dell EMC Isilon
2017-12-12 17:25:25 +00:00
markj
fd47853c2a Decrement sc_writes when BIO_DELETE requests complete.
Otherwise a gmirror that has received a BIO_DELETE request will never be
marked clean (unless sc_writes overflows).

MFC after:	1 week
Sponsored by:	Dell EMC Isilon
2017-12-12 17:24:30 +00:00
eugen
748192b1b0 geom_raid (RAID5): do not lose bp->bio_error, keep it in pbp->bio_error
and return it by passing to g_raid_iodone()

Approved by:	mav (mentor)
MFC after:	3 days
2017-12-07 20:09:17 +00:00
eugen
5d8549519a Fix use-after-free that sometimes results in a garbage returned
instead of right error code after requests to SINGLE/CONCAT volumes, f.e:

# dd if=/dev/raid/r0 bs=512 of=/dev/null
dd: /dev/raid/r0: Unknown error: -559038242

Reviewed by:	avg (mentor), mav (mentor)
MFC after:	3 days
2017-12-07 05:55:18 +00:00
imp
6bc680b3ff When building standalone, include stand.h rather than the kernel
includes or the userland includes.

Sponsored by: Netflix
2017-12-05 21:37:32 +00:00
imp
ff37b2b458 We don't need both _STAND and _STANDALONE. There's more places that
use _STANDALONE, so change the former to the latter.

Sponsored by: Netflix
2017-12-02 00:07:09 +00:00
markj
66ff112aae Update gmirror metadata less frequently when synchronizing.
We periodically record synchronization progress in the metadata
block of the disk being synchronized; this allows an interrupted
synchronization to be resumed. However, the frequency of these
updates heavily pessimized synchronization time on some media. This
change modifies gmirror to update metadata based on a time period,
and adds a sysctl to control that period. The default value results
in a much lower update frequency and increases the completion time
for an interrupted rebuild only marginally.

Reported by:	Andre Albsmeier <andre@fbsd.e4m.org>
MFC after:	3 weeks
2017-11-30 20:36:29 +00:00
pfg
a82e3a8b24 sys/geom: adoption of SPDX licensing ID tags.
Mainly focus on files that use BSD 2-Clause license, however the tool I
was using misidentified many licenses so this was mostly a manual - error
prone - task.

The Software Package Data Exchange (SPDX) group provides a specification
to make it easier for automated tools to detect and summarize well known
opensource licenses. We are gradually adopting the specification, noting
that the tags are considered only advisory and do not, in any way,
superceed or replace the license texts.
2017-11-27 15:17:37 +00:00
markj
36a1a14004 Allow kern.geom.mirror.debug to be negative.
A negative value can be used to suppress all prints from the gmirror
kernel code, which can be useful when attempting to trigger race
conditions using stress tests.

MFC after:	1 week
2017-11-23 14:07:52 +00:00
imp
8b5ceba282 While the EFI spec allows numbers to be in many forms, libefivar
produces hex numbers for the dsn. Since that come is from EDK2, change
this for symmetry, by generating the dsn as a hex number.

Noticed by: gpart list | grep efimedia | awk -F: '{print $2;}' | \
	sed -e 's/^ *//g;s/,,/,/' | grep MBR | efidp -p | efidp -f
Sponsored by: Netflix
2017-11-21 06:12:21 +00:00
imp
a69327a6a6 Remove trailing whitespace (one I just introduced and a bunch of
others in the same directory).

Sponsored by: Netflix
2017-11-21 05:42:13 +00:00
imp
b014d8a124 Implement efi media tagging for MBR partitioning types.
Sponsored by: Netflix
2017-11-21 05:35:21 +00:00
pfg
9da7bdde06 spdx: initial adoption of licensing ID tags.
The Software Package Data Exchange (SPDX) group provides a specification
to make it easier for automated tools to detect and summarize well known
opensource licenses. We are gradually adopting the specification, noting
that the tags are considered only advisory and do not, in any way,
superceed or replace the license texts.

Special thanks to Wind River for providing access to "The Duke of
Highlander" tool: an older (2014) run over FreeBSD tree was useful as a
starting point.

Initially, only tag files that use BSD 4-Clause "Original" license.

RelNotes:	yes
Differential Revision:	https://reviews.freebsd.org/D13133
2017-11-18 14:26:50 +00:00
avg
35c36e666a geom_slice: fix r325227, protect against multiple calls to g_slice_free
This geom does not immediately detach its consumer relying on the
wither-washer to do that.  Since that happens asynchronously we may get
additional spoiling events.  So, we need to account for that.

There are multiple options for fixing this issue like detaching
immediately or checking for G_CF_ORPHAN in g_slice_spoiled().
The most reliable and least intrusive fix seems to be setting
geom->softc to NULL on the first call and checking for NULL on
subsequent calls.  This is something that the code did before r325227.

Reported by:	David Wolfskill <david@catwhisker.org>,
		O. Hartmann <o.hartmann@walstatt.org>
Tested by:	David Wolfskill <david@catwhisker.org> (earlier version)
Discussed with:	mav
MFC after:	1 week
X-MFC with:	r325227
2017-11-01 10:53:10 +00:00
avg
04e7093fe7 geom_slice: do not destroy softc until providers are gone
At present, g_slice_orphan and g_slice_spoiled destroy the softc
(struct g_slicer) even before calling g_wither_geom, so there can
be active and incoming io requests at that time and g_slice_start
can access the softc.

This commit changes the code to destroy the softc only after all
providers are closed.

While there, a couple of small cleanups.

Reported by:	Ben RUBSON <ben.rubson@gmail.com>
Tested by:	Ben RUBSON <ben.rubson@gmail.com>
Reviewed by:	mav, smh (earlier version)
MFC after:	2 weeks
Sponsored by:	Panzura
Differential Revision: https://reviews.freebsd.org/D12809
2017-10-31 10:10:13 +00:00
trasz
112f0f8958 Add back missing MTX_DEF, it still needs to be there.
(Although it's defined to be 0, so there's no functional change.)

Reported by:	glebius
MFC after:	2 weeks
2017-10-29 12:03:06 +00:00
markj
7dbd75884a Fix a lock leak in g_mirror_destroy().
g_mirror_destroy() is supposed to unlock the softc before indicating
success, but it wasn't doing so if the caller raced with another
thread destroying the mirror.

MFC after:	1 week
Sponsored by:	Dell EMC Isilon
2017-10-27 17:05:14 +00:00
trasz
2589d9ccaf Make gmountver(8) use direct dispatch.
MFC after:	2 weeks
2017-10-26 10:18:31 +00:00
trasz
094f4d7259 Make gmountver(8) use G_PF_ACCEPT_UNMAPPED.
MFC after:	2 weeks
2017-10-26 09:29:35 +00:00
markj
a049a758b2 Add support for compressed kernel dumps.
When using a kernel built with the GZIO config option, dumpon -z can be
used to configure gzip compression using the in-kernel copy of zlib.
This is useful on systems with large amounts of RAM, which require a
correspondingly large dump device. Recovery of compressed dumps is also
faster since fewer bytes need to be copied from the dump device.

Because we have no way of knowing the final size of a compressed dump
until it is written, the kernel will always attempt to dump when
compression is configured, regardless of the dump device size. If the
dump is aborted because we run out of space, an error is reported on
the console.

savecore(8) is modified to handle compressed dumps and save them to
vmcore.<index>.gz, as it does when given the -z option.

A new rc.conf variable, dumpon_flags, is added. Its value is added to
the boot-time dumpon(8) invocation that occurs when a dump device is
configured in rc.conf.

Reviewed by:	cem (earlier version)
Discussed with:	def, rgrimes
Relnotes:	yes
Sponsored by:	Dell EMC Isilon
Differential Revision:	https://reviews.freebsd.org/D11723
2017-10-25 00:51:00 +00:00
asomers
5719c10d16 Display rotation rate and TRIM/UNMAP support in diskinfo(8)
Bump __FreeBSD_version due to the expansion of struct diocgattr_arg.

Reviewed by:	mav, allanjude, imp
MFC after:	3 weeks
Relnotes:	yes
Sponsored by:	Spectra Logic Corp
Differential Revision:	https://reviews.freebsd.org/D12578
2017-10-04 15:09:49 +00:00
trasz
6de6f3064c Don't destroy gmountver(8) devices on shutdown, unless they are orphaned.
Otherwise we would fail to sync the filesystem on reboot.

MFC after:	2 weeks
Sponsored by:	DARPA, AFRL
2017-10-04 12:25:39 +00:00
trasz
ad7945e0ab Clear G_CF_ORPHAN when attaching. This fixes cases where the same
GEOM consumer can be orphaned, and then reattach to another provider.

From a user point of view, this makes gmountver(4) work again.

Reviewed by:	avg, mav
MFC after:	2 weeks
Sponsored by:	DARPA, AFRL
Differential Revision:	https://reviews.freebsd.org/D12228
2017-10-02 11:57:00 +00:00
cem
1ff2951300 g_resize_provider_event: Do not invoke orphan method twice
Like r266444, g_resize_provider_event can attempt to orphan an already
orphaned geom_dev consumer.  This will cause a panic in g_dev_orphan.  Apply
the same fix as was applied to g_orphan_register.

Reviewed by:	ae
Sponsored by:	Dell EMC Isilon
Differential Revision:	https://reviews.freebsd.org/D12469
2017-09-24 19:59:26 +00:00
avg
78c53bec48 gmirror: treat ENXIO as disk disconnect, not media error
In theory, all data access errors mean that a member is out of sync
at most.  But they were treated as more serious errors to avoid the
situation where a flaky disk gets repeatedly disconnected, re-synchronized,
reconnected and then disconnected again.

ENXIO is a special error that means that the member disk disappeared,
so it should get the same handling as the GEOM orphaning event.
There is a better chance that when the disk is reconnected, it will be
a good member again.

When ENXIO happens on a read we use the exisiting G_MIRROR_BUMP_SYNCID
mechanism which means that the mirror's syncid is increased as soon
as there is a write to the mirror.  That's because no data has got out
of sync yet, but the problematic memeber is disconnected, so the future
write will make it stale.

When ENXIO happens on a write we use a new G_MIRROR_BUMP_SYNCID_NOW
mechanism which means that we update the mirror metadata as soon as
possible because the problematic memeber is already behind.

Reviewed by:	markj, imp
MFC after:	3 weeks
Differential Revision: https://reviews.freebsd.org/D9463
2017-09-15 13:57:08 +00:00
cem
edd80ade51 Fix information leak in geli(8) integrity mode
In integrity mode, a larger logical sector (e.g., 4096 bytes) spans several
physical sectors (e.g., 512 bytes) on the backing device.  Due to hash
overhead, a 4096 byte logical sector takes 8.5625 512-byte physical sectors.
This means that only 288 bytes (256 data + 32 hash) of the last 512 byte
sector are used.

The memory allocation used to store the encrypted data to be written to the
physical sectors comes from malloc(9) and does not use M_ZERO.

Previously, nothing initialized the final physical sector backing each
logical sector, aside from the hash + encrypted data portion.  So 224 bytes
of kernel heap memory was leaked to every block :-(.

This patch addresses the issue by initializing the trailing portion of the
physical sector in every logical sector to zeros before use.  A much simpler
but higher overhead fix would be to tag the entire allocation M_ZERO.

PR:		222077
Reported by:	Maxim Khitrov <max AT mxcrypt.com>
Reviewed by:	emaste
Security:	yes
Sponsored by:	Dell EMC Isilon
Differential Revision:	https://reviews.freebsd.org/D12272
2017-09-09 01:41:01 +00:00
imp
d38c6face9 The hard drive media device path contains the size of the partition,
not its end. This makes the GEOM efimedia attribute match the
FreeBSD:Boot1Device environment variable now.

Sponsored by: Netflix
2017-09-02 07:04:06 +00:00
imp
d82708bf65 Add efimedia attribute for all GPT partitions.
Sposnored by: Netflix
Differential Revision: https://reviews.freebsd.org/D12206
2017-09-01 17:55:25 +00:00
kib
fd3df44d57 Let g_access() log the actual error number.
Submitted by:	 Fabian Keil <fk@fabiankeil.de>
PR:	221855
MFC after:	1 week
2017-08-27 12:24:25 +00:00
oshogbo
53bbe84593 Hide length of geli passphrase during boot.
Introduce additional flag to the geli which allows to restore previous
behavior.

Reviewed by:	AllanJude@, cem@ (previous version)
MFC:		1 month
Relnotes:	yes
Differential Revision:	https://reviews.freebsd.org/D11751
2017-08-26 14:07:24 +00:00
mckusick
12015c2c0a When read requests are sent from a filesystem running above g_journal,
the g_journal level needs to check whether it is holding a newer
copy of the block than that which exists on the disk. If so, it
needs to return its copy. If not, it should pass the request down
to the disk to fulfill. It currently considers six queues:

0) delayed queue,
1) unsent (current queue),
2) in-flight to the journal (flush queue),
3) active journal (active queue),
4) inactive journal (inactive queue), and
5) inflight to the disk (copy queue).

Checking on two of these queues is unnecessary:

0) The delayed requests should not be used for reads because they
   have not yet been entered into the journal, so their value should
   reflect the disk contents, not the future contents that are not
   yet committed.

2) Because all the bio's in the flush queue are also found on the
   active queue, there is no need to inspect the flush queue for
   reads since they will be found when searching the active queue.

Submitted by: Dr. Andreas Longwitz <longwitz@incore.de>
Discussed with: kib
MFC after: 1 week
2017-08-13 18:09:22 +00:00
mckusick
9732028639 Eliminate a variable that is only ever set.
Submitted by: Dr. Andreas Longwitz <longwitz@incore.de>
Discussed with: kib
MFC after: 1 week
2017-08-13 18:06:38 +00:00