* [PATCH 2/6] block: Add part_stat_read_accum to read across field entries.
From: Tejun Heo @ 2018-07-18 11:47 UTC (permalink / raw)
To: axboe
Cc: michaelcallahan, newella, linux-block, linux-kernel, kernel-team,
linux-api, Tejun Heo
In-Reply-To: <20180718114741.2580313-1-tj@kernel.org>
From: Michael Callahan <michaelcallahan@fb.com>
Add a part_stat_read_accum macro to genhd.h to read and sum across
field entries. For example to sum up the number read and write
sectors completed. In addition to being ar reasonable cleanup by
itself this will make it easier to add new stat fields in the future.
tj: Refreshed on top of v4.17.
Signed-off-by: Michael Callahan <michaelcallahan@fb.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
drivers/block/drbd/drbd_receiver.c | 3 +--
drivers/block/drbd/drbd_worker.c | 4 +---
drivers/md/md.c | 3 +--
include/linux/genhd.h | 4 ++++
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
index a36a30795c43..75f6b47169e6 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -2674,8 +2674,7 @@ bool drbd_rs_c_min_rate_throttle(struct drbd_device *device)
if (c_min_rate == 0)
return false;
- curr_events = (int)part_stat_read(&disk->part0, sectors[0]) +
- (int)part_stat_read(&disk->part0, sectors[1]) -
+ curr_events = (int)part_stat_read_accum(&disk->part0, sectors) -
atomic_read(&device->rs_sect_ev);
if (atomic_read(&device->ap_actlog_cnt)
diff --git a/drivers/block/drbd/drbd_worker.c b/drivers/block/drbd/drbd_worker.c
index 5e793dd7adfb..b8f77e83d456 100644
--- a/drivers/block/drbd/drbd_worker.c
+++ b/drivers/block/drbd/drbd_worker.c
@@ -1690,9 +1690,7 @@ void drbd_rs_controller_reset(struct drbd_device *device)
atomic_set(&device->rs_sect_in, 0);
atomic_set(&device->rs_sect_ev, 0);
device->rs_in_flight = 0;
- device->rs_last_events =
- (int)part_stat_read(&disk->part0, sectors[0]) +
- (int)part_stat_read(&disk->part0, sectors[1]);
+ device->rs_last_events = (int)part_stat_read_accum(&disk->part0, sectors);
/* Updating the RCU protected object in place is necessary since
this function gets called from atomic context.
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 994aed2f9dff..dabe36723d60 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -8046,8 +8046,7 @@ static int is_mddev_idle(struct mddev *mddev, int init)
rcu_read_lock();
rdev_for_each_rcu(rdev, mddev) {
struct gendisk *disk = rdev->bdev->bd_contains->bd_disk;
- curr_events = (int)part_stat_read(&disk->part0, sectors[0]) +
- (int)part_stat_read(&disk->part0, sectors[1]) -
+ curr_events = (int)part_stat_read_accum(&disk->part0, sectors) -
atomic_read(&disk->sync_io);
/* sync IO will cause sync_io to increase before the disk_stats
* as sync_io is counted when a request starts, and
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 6cb8a5789668..19f36fa10995 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -353,6 +353,10 @@ static inline void free_part_stats(struct hd_struct *part)
#endif /* CONFIG_SMP */
+#define part_stat_read_accum(part, field) \
+ (part_stat_read(part, field[0]) + \
+ part_stat_read(part, field[1]))
+
#define part_stat_add(cpu, part, field, addnd) do { \
__part_stat_add((cpu), (part), field, addnd); \
if ((part)->partno) \
--
2.17.1
^ permalink raw reply related
* [PATCH 1/6] block: make bdev_ops->rw_page() take a REQ_OP instead of bool
From: Tejun Heo @ 2018-07-18 11:47 UTC (permalink / raw)
To: axboe
Cc: michaelcallahan, newella, linux-block, linux-kernel, kernel-team,
linux-api, Tejun Heo, Mike Christie, Minchan Kim, Dan Williams
In-Reply-To: <20180718114741.2580313-1-tj@kernel.org>
c11f0c0b5bb9 ("block/mm: make bdev_ops->rw_page() take a bool for
read/write") replaced @op with boolean @is_write, which limited the
amount of information going into ->rw_page() and more importantly
page_endio(), which removed the need to expose block internals to mm.
Unfortunately, we want to track discards separately and @is_write
isn't enough information. This patch updates bdev_ops->rw_page() to
take REQ_OP instead but leaves page_endio() to take bool @is_write.
This allows the block part of operations to have enough information
while not leaking it to mm.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Mike Christie <mchristi@redhat.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Dan Williams <dan.j.williams@intel.com>
---
drivers/block/brd.c | 14 +++++++-------
drivers/block/zram/zram_drv.c | 16 ++++++++--------
drivers/nvdimm/btt.c | 12 ++++++------
drivers/nvdimm/pmem.c | 13 ++++++-------
fs/block_dev.c | 6 ++++--
fs/mpage.c | 4 ++--
include/linux/blkdev.h | 2 +-
7 files changed, 34 insertions(+), 33 deletions(-)
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index bb976598ee43..df8103dd40ac 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -254,20 +254,20 @@ static void copy_from_brd(void *dst, struct brd_device *brd,
* Process a single bvec of a bio.
*/
static int brd_do_bvec(struct brd_device *brd, struct page *page,
- unsigned int len, unsigned int off, bool is_write,
+ unsigned int len, unsigned int off, unsigned int op,
sector_t sector)
{
void *mem;
int err = 0;
- if (is_write) {
+ if (op_is_write(op)) {
err = copy_to_brd_setup(brd, sector, len);
if (err)
goto out;
}
mem = kmap_atomic(page);
- if (!is_write) {
+ if (!op_is_write(op)) {
copy_from_brd(mem + off, brd, sector, len);
flush_dcache_page(page);
} else {
@@ -296,7 +296,7 @@ static blk_qc_t brd_make_request(struct request_queue *q, struct bio *bio)
int err;
err = brd_do_bvec(brd, bvec.bv_page, len, bvec.bv_offset,
- op_is_write(bio_op(bio)), sector);
+ bio_op(bio), sector);
if (err)
goto io_error;
sector += len >> SECTOR_SHIFT;
@@ -310,15 +310,15 @@ static blk_qc_t brd_make_request(struct request_queue *q, struct bio *bio)
}
static int brd_rw_page(struct block_device *bdev, sector_t sector,
- struct page *page, bool is_write)
+ struct page *page, unsigned int op)
{
struct brd_device *brd = bdev->bd_disk->private_data;
int err;
if (PageTransHuge(page))
return -ENOTSUPP;
- err = brd_do_bvec(brd, page, PAGE_SIZE, 0, is_write, sector);
- page_endio(page, is_write, err);
+ err = brd_do_bvec(brd, page, PAGE_SIZE, 0, op, sector);
+ page_endio(page, op_is_write(op), err);
return err;
}
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 7436b2d27fa3..78c29044684a 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1274,17 +1274,17 @@ static void zram_bio_discard(struct zram *zram, u32 index,
* Returns 1 if IO request was successfully submitted.
*/
static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
- int offset, bool is_write, struct bio *bio)
+ int offset, unsigned int op, struct bio *bio)
{
unsigned long start_time = jiffies;
- int rw_acct = is_write ? REQ_OP_WRITE : REQ_OP_READ;
+ int rw_acct = op_is_write(op) ? REQ_OP_WRITE : REQ_OP_READ;
struct request_queue *q = zram->disk->queue;
int ret;
generic_start_io_acct(q, rw_acct, bvec->bv_len >> SECTOR_SHIFT,
&zram->disk->part0);
- if (!is_write) {
+ if (!op_is_write(op)) {
atomic64_inc(&zram->stats.num_reads);
ret = zram_bvec_read(zram, bvec, index, offset, bio);
flush_dcache_page(bvec->bv_page);
@@ -1300,7 +1300,7 @@ static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
zram_slot_unlock(zram, index);
if (unlikely(ret < 0)) {
- if (!is_write)
+ if (!op_is_write(op))
atomic64_inc(&zram->stats.failed_reads);
else
atomic64_inc(&zram->stats.failed_writes);
@@ -1338,7 +1338,7 @@ static void __zram_make_request(struct zram *zram, struct bio *bio)
bv.bv_len = min_t(unsigned int, PAGE_SIZE - offset,
unwritten);
if (zram_bvec_rw(zram, &bv, index, offset,
- op_is_write(bio_op(bio)), bio) < 0)
+ bio_op(bio), bio) < 0)
goto out;
bv.bv_offset += bv.bv_len;
@@ -1390,7 +1390,7 @@ static void zram_slot_free_notify(struct block_device *bdev,
}
static int zram_rw_page(struct block_device *bdev, sector_t sector,
- struct page *page, bool is_write)
+ struct page *page, unsigned int op)
{
int offset, ret;
u32 index;
@@ -1414,7 +1414,7 @@ static int zram_rw_page(struct block_device *bdev, sector_t sector,
bv.bv_len = PAGE_SIZE;
bv.bv_offset = 0;
- ret = zram_bvec_rw(zram, &bv, index, offset, is_write, NULL);
+ ret = zram_bvec_rw(zram, &bv, index, offset, op, NULL);
out:
/*
* If I/O fails, just return error(ie, non-zero) without
@@ -1429,7 +1429,7 @@ static int zram_rw_page(struct block_device *bdev, sector_t sector,
switch (ret) {
case 0:
- page_endio(page, is_write, 0);
+ page_endio(page, op_is_write(op), 0);
break;
case 1:
ret = 0;
diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index 85de8053aa34..0360c015f658 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -1423,11 +1423,11 @@ static int btt_write_pg(struct btt *btt, struct bio_integrity_payload *bip,
static int btt_do_bvec(struct btt *btt, struct bio_integrity_payload *bip,
struct page *page, unsigned int len, unsigned int off,
- bool is_write, sector_t sector)
+ unsigned int op, sector_t sector)
{
int ret;
- if (!is_write) {
+ if (!op_is_write(op)) {
ret = btt_read_pg(btt, bip, page, off, sector, len);
flush_dcache_page(page);
} else {
@@ -1464,7 +1464,7 @@ static blk_qc_t btt_make_request(struct request_queue *q, struct bio *bio)
}
err = btt_do_bvec(btt, bip, bvec.bv_page, len, bvec.bv_offset,
- op_is_write(bio_op(bio)), iter.bi_sector);
+ bio_op(bio), iter.bi_sector);
if (err) {
dev_err(&btt->nd_btt->dev,
"io error in %s sector %lld, len %d,\n",
@@ -1483,16 +1483,16 @@ static blk_qc_t btt_make_request(struct request_queue *q, struct bio *bio)
}
static int btt_rw_page(struct block_device *bdev, sector_t sector,
- struct page *page, bool is_write)
+ struct page *page, unsigned int op)
{
struct btt *btt = bdev->bd_disk->private_data;
int rc;
unsigned int len;
len = hpage_nr_pages(page) * PAGE_SIZE;
- rc = btt_do_bvec(btt, NULL, page, len, 0, is_write, sector);
+ rc = btt_do_bvec(btt, NULL, page, len, 0, op, sector);
if (rc == 0)
- page_endio(page, is_write, 0);
+ page_endio(page, op_is_write(op), 0);
return rc;
}
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 8b1fd7f1a224..dd17acd8fe68 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -120,7 +120,7 @@ static blk_status_t read_pmem(struct page *page, unsigned int off,
}
static blk_status_t pmem_do_bvec(struct pmem_device *pmem, struct page *page,
- unsigned int len, unsigned int off, bool is_write,
+ unsigned int len, unsigned int off, unsigned int op,
sector_t sector)
{
blk_status_t rc = BLK_STS_OK;
@@ -131,7 +131,7 @@ static blk_status_t pmem_do_bvec(struct pmem_device *pmem, struct page *page,
if (unlikely(is_bad_pmem(&pmem->bb, sector, len)))
bad_pmem = true;
- if (!is_write) {
+ if (!op_is_write(op)) {
if (unlikely(bad_pmem))
rc = BLK_STS_IOERR;
else {
@@ -180,8 +180,7 @@ static blk_qc_t pmem_make_request(struct request_queue *q, struct bio *bio)
do_acct = nd_iostat_start(bio, &start);
bio_for_each_segment(bvec, bio, iter) {
rc = pmem_do_bvec(pmem, bvec.bv_page, bvec.bv_len,
- bvec.bv_offset, op_is_write(bio_op(bio)),
- iter.bi_sector);
+ bvec.bv_offset, bio_op(bio), iter.bi_sector);
if (rc) {
bio->bi_status = rc;
break;
@@ -198,13 +197,13 @@ static blk_qc_t pmem_make_request(struct request_queue *q, struct bio *bio)
}
static int pmem_rw_page(struct block_device *bdev, sector_t sector,
- struct page *page, bool is_write)
+ struct page *page, unsigned int op)
{
struct pmem_device *pmem = bdev->bd_queue->queuedata;
blk_status_t rc;
rc = pmem_do_bvec(pmem, page, hpage_nr_pages(page) * PAGE_SIZE,
- 0, is_write, sector);
+ 0, op, sector);
/*
* The ->rw_page interface is subtle and tricky. The core
@@ -213,7 +212,7 @@ static int pmem_rw_page(struct block_device *bdev, sector_t sector,
* caused by double completion.
*/
if (rc == 0)
- page_endio(page, is_write, 0);
+ page_endio(page, op_is_write(op), 0);
return blk_status_to_errno(rc);
}
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 0dd87aaeb39a..496fb51a1e1a 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -665,7 +665,8 @@ int bdev_read_page(struct block_device *bdev, sector_t sector,
result = blk_queue_enter(bdev->bd_queue, 0);
if (result)
return result;
- result = ops->rw_page(bdev, sector + get_start_sect(bdev), page, false);
+ result = ops->rw_page(bdev, sector + get_start_sect(bdev), page,
+ REQ_OP_READ);
blk_queue_exit(bdev->bd_queue);
return result;
}
@@ -703,7 +704,8 @@ int bdev_write_page(struct block_device *bdev, sector_t sector,
return result;
set_page_writeback(page);
- result = ops->rw_page(bdev, sector + get_start_sect(bdev), page, true);
+ result = ops->rw_page(bdev, sector + get_start_sect(bdev), page,
+ REQ_OP_WRITE);
if (result) {
end_page_writeback(page);
} else {
diff --git a/fs/mpage.c b/fs/mpage.c
index b7e7f570733a..b73638db9866 100644
--- a/fs/mpage.c
+++ b/fs/mpage.c
@@ -51,8 +51,8 @@ static void mpage_end_io(struct bio *bio)
bio_for_each_segment_all(bv, bio, i) {
struct page *page = bv->bv_page;
- page_endio(page, op_is_write(bio_op(bio)),
- blk_status_to_errno(bio->bi_status));
+ page_endio(page, bio_op(bio),
+ blk_status_to_errno(bio->bi_status));
}
bio_put(bio);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 1939ed95f936..331a6cb8805f 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1943,7 +1943,7 @@ static inline bool integrity_req_gap_front_merge(struct request *req,
struct block_device_operations {
int (*open) (struct block_device *, fmode_t);
void (*release) (struct gendisk *, fmode_t);
- int (*rw_page)(struct block_device *, sector_t, struct page *, bool);
+ int (*rw_page)(struct block_device *, sector_t, struct page *, unsigned int);
int (*ioctl) (struct block_device *, fmode_t, unsigned, unsigned long);
int (*compat_ioctl) (struct block_device *, fmode_t, unsigned, unsigned long);
unsigned int (*check_events) (struct gendisk *disk,
--
2.17.1
^ permalink raw reply related
* [PATCHSET v3] block: Separating discards from writes in Linux IO statistics
From: Tejun Heo @ 2018-07-18 11:47 UTC (permalink / raw)
To: axboe
Cc: michaelcallahan, newella, linux-block, linux-kernel, kernel-team,
linux-api
Hello,
Changes from v2: Refreshed on top of for-4.19/block.
This patchset was originally posted by Michael Callahan.
https://marc.info/?l=linux-block&m=146541910129172&w=2
The patchset is refreshed on top of the current git master
v4.17-1306-g716a685 and a patch was added to add discard stats for
cgroup io.stat. The original patchset description from Michael
follows.
This patch set separates block layer statistics for discards from
writes. Discards are currently bundled with writes in the various
/sys/block/*/stat files as well as in /proc/diskstats. However
discards are nearly always used to mark storage that is no longer in
use. There are many reasons having discard not counted with writes is
useful:
1) For many non volatile memory devices it is just nice to know
that discards are enabled and working properly.
2) Discards have different performance characteristics than
writes. They are generally much faster and larger and bundling
them makes performance statistics less meaningful.
3) Discards are not writes in terms of tracking device lifetime.
If a device supports six device writes per day it is nice to know
how many writes have actually been written to the device as
discards do not count against that total.
Separation of discard statistics is accomplished by expanding the
struct diskstat arrays to 3 entries for STAT_READ, STAT_WRITE,
and STAT_DISCARD. A new rw_stat_group function is then used to
convert from rw_flags (cmd_flags from requests, bi_rw from bios)
into the appropriate stat group which is then tracked as before.
Lastly the new statistics are appended to the current
/sys/bloc/*/stat and /proc/diskstats on output such that they are
the last four entries of each. These are analogous to the four
read and write statistics.
* Number of discard ios completed
* Number of discard ios merged
* Number of discard sectors completed
* Milliseconds spent on discard requests.
[before ~]# cat sys/block/nvme0/stat
296550701 0 2372405688 67317193 19672752 0 7972237312
9375167 0 2787238 79718726
[after ~]# cat sys/block/nvme0/stat
296550701 0 2372405688 67317193 18034352 0 4616794112
9125902 0 2787238 79718726 1638400 0 3355443200
249265
Note that the discards have moved out of the write fields to the
end and that the write fields are now smaller by the difference.
Adding the new statistics to the end of /sys/block/*/stat and
/proc/diskstats is backwards compatible with both iostat and
vmstat which pick up just the old fields:
[root@after ~]# iostat -x
Linux 4.5.0_68319_ge5065f4-dirty (##hostname###) 05/17/2016
_x86_64_ (48 CPU)
avg-cpu: %user %nice %system %iowait %steal %idle
0.41 0.00 0.23 0.01 0.00 99.35
Device: rrqm/s wrqm/s r/s w/s rsec/s wsec/s
avgrq-sz avgqu-sz await svctm %util
sda 0.01 7.50 0.20 3.61 16.65 708.41
190.03 0.09 22.53 1.16 0.44
nvme0n1 0.00 0.00 587.03 35.70 4696.25 9139.08
22.22 0.16 0.24 0.01 0.55
[root@after ~]# vmstat -d
disk- ------------reads------------ ------------writes----------- -----IO------
total merged sectors ms total merged sectors ms cur sec
ram0 0 0 0 0 0 0 0 0 0 0
ram1 0 0 0 0 0 0 0 0 0 0
ram2 0 0 0 0 0 0 0 0 0 0
ram3 0 0 0 0 0 0 0 0 0 0
ram4 0 0 0 0 0 0 0 0 0 0
ram5 0 0 0 0 0 0 0 0 0 0
ram6 0 0 0 0 0 0 0 0 0 0
ram7 0 0 0 0 0 0 0 0 0 0
ram8 0 0 0 0 0 0 0 0 0 0
ram9 0 0 0 0 0 0 0 0 0 0
ram10 0 0 0 0 0 0 0 0 0 0
ram11 0 0 0 0 0 0 0 0 0 0
ram12 0 0 0 0 0 0 0 0 0 0
ram13 0 0 0 0 0 0 0 0 0 0
ram14 0 0 0 0 0 0 0 0 0 0
ram15 0 0 0 0 0 0 0 0 0 0
sda 102903 5247 8408420 47424 1820306 3782041 357153613 43320121
0 2228
nvme0n1 145633279 0 1165066312 18981333 13107200 0
3355443200 6663655 0 1796
loop0 0 0 0 0 0 0 0 0 0 0
loop1 0 0 0 0 0 0 0 0 0 0
[chop rest of loop devices]
[root@after ~]# cat /sys/fs/cgroup/user.slice/io.stat
8:0 rbytes=3534848 wbytes=4096 rios=723 wios=1 dbytes=20592091136 dios=16189
This patchset contains the following six patches.
0001-block-make-bdev_ops-rw_page-take-a-REQ_OP-instead-of.patch
0002-block-Add-part_stat_read_accum-to-read-across-field-.patch
0003-block-Define-and-use-STAT_READ-and-STAT_WRITE.patch
0004-block-Add-and-use-op_stat_group-for-indexing-disk_st.patch
0005-block-Track-DISCARD-statistics-and-output-them-in-st.patch
0006-blkcg-Track-DISCARD-statistics-and-output-them-in-cg.patch
and also available in the following git branch.
git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git block-discard-stat-v3
diffstat follows. Thanks.
Documentation/ABI/testing/procfs-diskstats | 10 ++++++++++
Documentation/admin-guide/cgroup-v2.rst | 10 ++++++----
Documentation/block/stat.txt | 28 ++++++++++++++++------------
Documentation/iostats.txt | 15 +++++++++++++++
block/bio.c | 16 +++++++++-------
block/blk-cgroup.c | 14 ++++++++++----
block/blk-core.c | 12 ++++++------
block/genhd.c | 29 ++++++++++++++++++-----------
block/partition-generic.c | 25 +++++++++++++++----------
drivers/block/brd.c | 14 +++++++-------
drivers/block/drbd/drbd_receiver.c | 3 +--
drivers/block/drbd/drbd_req.c | 4 ++--
drivers/block/drbd/drbd_worker.c | 4 +---
drivers/block/rsxx/dev.c | 6 +++---
drivers/block/zram/zram_drv.c | 19 +++++++++----------
drivers/lightnvm/pblk-cache.c | 5 +++--
drivers/lightnvm/pblk-read.c | 5 +++--
drivers/md/bcache/request.c | 13 +++++--------
drivers/md/dm.c | 6 ++++--
drivers/md/md.c | 8 ++++----
drivers/nvdimm/btt.c | 12 ++++++------
drivers/nvdimm/nd.h | 7 +++----
drivers/nvdimm/pmem.c | 13 ++++++-------
fs/block_dev.c | 6 ++++--
fs/ext4/super.c | 5 +++--
fs/ext4/sysfs.c | 6 ++++--
fs/f2fs/f2fs.h | 2 +-
fs/f2fs/super.c | 3 ++-
fs/mpage.c | 4 ++--
include/linux/bio.h | 4 ++--
include/linux/blk-cgroup.h | 5 ++++-
include/linux/blk_types.h | 20 ++++++++++++++++++++
include/linux/blkdev.h | 2 +-
include/linux/genhd.h | 14 ++++++++++----
34 files changed, 215 insertions(+), 134 deletions(-)
--
tejun
^ permalink raw reply
* Re: [RFC PATCH v2 16/27] mm: Modify can_follow_write_pte/pmd for shadow stack
From: Dave Hansen @ 2018-07-17 23:15 UTC (permalink / raw)
To: Yu-cheng Yu, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Cyrill Gorcunov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Peter Zijlstra <pet>
In-Reply-To: <1531868610.3541.21.camel@intel.com>
On 07/17/2018 04:03 PM, Yu-cheng Yu wrote:
> We need to find a way to differentiate "someone can write to this PTE"
> from "the write bit is set in this PTE".
Please think about this:
Should pte_write() tell us whether PTE.W=1, or should it tell us
that *something* can write to the PTE, which would include
PTE.W=0/D=1?
^ permalink raw reply
* Re: [RFC PATCH v2 16/27] mm: Modify can_follow_write_pte/pmd for shadow stack
From: Dave Hansen @ 2018-07-17 23:11 UTC (permalink / raw)
To: Yu-cheng Yu, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Cyrill Gorcunov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Peter Zijlstra <pet>
In-Reply-To: <1531868610.3541.21.camel@intel.com>
On 07/17/2018 04:03 PM, Yu-cheng Yu wrote:
> On Fri, 2018-07-13 at 11:26 -0700, Dave Hansen wrote:
>> On 07/11/2018 10:05 AM, Yu-cheng Yu wrote:
>>>
>>> My understanding is that we don't want to follow write pte if the page
>>> is shared as read-only. For a SHSTK page, that is (R/O + DIRTY_SW),
>>> which means the SHSTK page has not been COW'ed. Is that right?
>> Let's look at the code again:
>>
>>>
>>> -static inline bool can_follow_write_pte(pte_t pte, unsigned int flags)
>>> +static inline bool can_follow_write_pte(pte_t pte, unsigned int flags,
>>> + bool shstk)
>>> {
>>> + bool pte_cowed = shstk ? is_shstk_pte(pte):pte_dirty(pte);
>>> +
>>> return pte_write(pte) ||
>>> - ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte));
>>> + ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_cowed);
>>> }
>> This is another case where the naming of pte_*() is biting us vs. the
>> perversion of the PTE bits. The lack of comments and explanation inthe
>> patch is compounding the confusion.
>>
>> We need to find a way to differentiate "someone can write to this PTE"
>> from "the write bit is set in this PTE".
>>
>> In this particular hunk, we need to make it clear that pte_write() is
>> *never* true for shadowstack PTEs. In other words, shadow stack VMAs
>> will (should?) never even *see* a pte_write() PTE.
>>
>> I think this is a case where you just need to bite the bullet and
>> bifurcate can_follow_write_pte(). Just separate the shadowstack and
>> non-shadowstack parts.
>
> In case I don't understand the exact issue.
> What about the following.
>
> diff --git a/mm/gup.c b/mm/gup.c
> index fc5f98069f4e..45a0837b27f9 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -70,6 +70,12 @@ static inline bool can_follow_write_pte(pte_t pte, unsigned int flags)
> ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte));
> }
>
> +static inline bool can_follow_write_shstk_pte(pte_t pte, unsigned int flags)
> +{
> + return ((flags & FOLL_FORCE) && (flags & FOLL_COW) &&
> + is_shstk_pte(pte));
> +}
> +
> static struct page *follow_page_pte(struct vm_area_struct *vma,
> unsigned long address, pmd_t *pmd, unsigned int flags)
> {
> @@ -105,9 +111,16 @@ static struct page *follow_page_pte(struct vm_area_struct *vma,
> }
> if ((flags & FOLL_NUMA) && pte_protnone(pte))
> goto no_page;
> - if ((flags & FOLL_WRITE) && !can_follow_write_pte(pte, flags)) {
> - pte_unmap_unlock(ptep, ptl);
> - return NULL;
> + if (flags & FOLL_WRITE) {
> + if (is_shstk_mapping(vma->vm_flags)) {
> + if (!can_follow_write_shstk_pte(pte, flags)) {
> + pte_unmap_unlock(ptep, ptl);
> + return NULL;
> + }
> + } else if (!can_follow_write_pte(pte, flags) {
> + pte_unmap_unlock(ptep, ptl);
> + return NULL;
> + }
That looks pretty horrible. :(
We need:
bool can_follow_write(vma, pte_t pte, unsigned int flags)
{
if (!is_shstk_mapping(vma->vm_flags)) {
// vanilla case here
} else {
// shadowstack case here
}
}
^ permalink raw reply
* Re: [RFC PATCH v2 16/27] mm: Modify can_follow_write_pte/pmd for shadow stack
From: Yu-cheng Yu @ 2018-07-17 23:03 UTC (permalink / raw)
To: Dave Hansen, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Cyrill Gorcunov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek
In-Reply-To: <45a85b01-e005-8cb6-af96-b23ce9b5fca7@linux.intel.com>
On Fri, 2018-07-13 at 11:26 -0700, Dave Hansen wrote:
> On 07/11/2018 10:05 AM, Yu-cheng Yu wrote:
> >
> > My understanding is that we don't want to follow write pte if the page
> > is shared as read-only. For a SHSTK page, that is (R/O + DIRTY_SW),
> > which means the SHSTK page has not been COW'ed. Is that right?
> Let's look at the code again:
>
> >
> > -static inline bool can_follow_write_pte(pte_t pte, unsigned int flags)
> > +static inline bool can_follow_write_pte(pte_t pte, unsigned int flags,
> > + bool shstk)
> > {
> > + bool pte_cowed = shstk ? is_shstk_pte(pte):pte_dirty(pte);
> > +
> > return pte_write(pte) ||
> > - ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte));
> > + ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_cowed);
> > }
> This is another case where the naming of pte_*() is biting us vs. the
> perversion of the PTE bits. The lack of comments and explanation inthe
> patch is compounding the confusion.
>
> We need to find a way to differentiate "someone can write to this PTE"
> from "the write bit is set in this PTE".
>
> In this particular hunk, we need to make it clear that pte_write() is
> *never* true for shadowstack PTEs. In other words, shadow stack VMAs
> will (should?) never even *see* a pte_write() PTE.
>
> I think this is a case where you just need to bite the bullet and
> bifurcate can_follow_write_pte(). Just separate the shadowstack and
> non-shadowstack parts.
In case I don't understand the exact issue.
What about the following.
diff --git a/mm/gup.c b/mm/gup.c
index fc5f98069f4e..45a0837b27f9 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -70,6 +70,12 @@ static inline bool can_follow_write_pte(pte_t pte, unsigned int flags)
((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte));
}
+static inline bool can_follow_write_shstk_pte(pte_t pte, unsigned int flags)
+{
+ return ((flags & FOLL_FORCE) && (flags & FOLL_COW) &&
+ is_shstk_pte(pte));
+}
+
static struct page *follow_page_pte(struct vm_area_struct *vma,
unsigned long address, pmd_t *pmd, unsigned int flags)
{
@@ -105,9 +111,16 @@ static struct page *follow_page_pte(struct vm_area_struct *vma,
}
if ((flags & FOLL_NUMA) && pte_protnone(pte))
goto no_page;
- if ((flags & FOLL_WRITE) && !can_follow_write_pte(pte, flags)) {
- pte_unmap_unlock(ptep, ptl);
- return NULL;
+ if (flags & FOLL_WRITE) {
+ if (is_shstk_mapping(vma->vm_flags)) {
+ if (!can_follow_write_shstk_pte(pte, flags)) {
+ pte_unmap_unlock(ptep, ptl);
+ return NULL;
+ }
+ } else if (!can_follow_write_pte(pte, flags) {
+ pte_unmap_unlock(ptep, ptl);
+ return NULL;
+ }
}
page = vm_normal_page(vma, address, pte);
^ permalink raw reply related
* Re: [RFC PATCH v2 16/27] mm: Modify can_follow_write_pte/pmd for shadow stack
From: Yu-cheng Yu @ 2018-07-17 23:00 UTC (permalink / raw)
To: Peter Zijlstra
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Cyrill Gorcunov, Dave Hansen,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek, Ravi
In-Reply-To: <20180711092951.GW2476@hirez.programming.kicks-ass.net>
On Wed, 2018-07-11 at 11:29 +0200, Peter Zijlstra wrote:
> On Tue, Jul 10, 2018 at 03:26:28PM -0700, Yu-cheng Yu wrote:
> >
> > There are three possible shadow stack PTE settings:
> >
> > Normal SHSTK PTE: (R/O + DIRTY_HW)
> > SHSTK PTE COW'ed: (R/O + DIRTY_HW)
> > SHSTK PTE shared as R/O data: (R/O + DIRTY_SW)
> I count _2_ distinct states there.
>
> >
> > Update can_follow_write_pte/pmd for the shadow stack.
> So the below disallows can_follow_write when shstk && _PAGE_DIRTY_SW,
> but this here Changelog doesn't explain why. Doesn't even get close.
Can we add the following to the log:
When a SHSTK PTE is shared, it is (R/O + DIRTY_SW); otherwise it is
(R/O + DIRTY_HW).
When we (FOLL_WRITE | FOLL_FORCE) on a SHSTK PTE, the following
must be true:
- It has been COW'ed at least once (FOLL_COW is set);
- It still is not shared, i.e. PTE is (R/O + DIRTY_HW);
>
> Also, the code is a right mess :/ Can't we try harder to not let this
> shadow stack stuff escape arch code.
We either check here if the VMA is SHSTK mapping or move the logic
to pte_dirty(). The latter would be less obvious. Or can we
create a can_follow_write_shstk_pte()?
Yu-cheng
^ permalink raw reply
* Re: [RESEND][PATCH v4 0/2] vfs: better dedupe permission check
From: Mark Fasheh @ 2018-07-17 20:14 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Al Viro, linux-fsdevel, linux-btrfs, linux-xfs, Adam Borowski,
David Sterba, Michael Kerrisk, linux-api
In-Reply-To: <20180717194818.GV32415@magnolia>
CCing Michael Kerrisk and linux-api
The patch at the end of this e-mail updates our man page for
ioctl_fideduperange to reflect the changes proposed in this patch series:
https://marc.info/?l=linux-fsdevel&m=153185457324037&w=2
Any feedback would be appreciated. Thanks in advance.
--Mark
On Tue, Jul 17, 2018 at 12:48:18PM -0700, Darrick J. Wong wrote:
> On Tue, Jul 17, 2018 at 12:09:04PM -0700, Mark Fasheh wrote:
> > Hi Al,
> >
> > The following patches fix a couple of issues with the permission check
> > we do in vfs_dedupe_file_range(). I sent them out for a few times now,
> > a changelog is attached. If they look ok to you, I'd appreciate them
> > being pushed upstream.
> >
> > You can get them from git if you like:
> >
> > git pull https://github.com/markfasheh/linux dedupe-perms
> >
> > I also have a set of patches against 4.17 if you prefer. The code and
> > testing are identical:
> >
> > git pull https://github.com/markfasheh/linux dedupe-perms-v4.17
> >
> >
> > The first patch expands our check to allow dedupe of a file if the
> > user owns it or otherwise would be allowed to write to it.
> >
> > Current behavior is that we'll allow dedupe only if:
> >
> > - the user is an admin (root)
> > - the user has the file open for write
> >
> > This makes it impossible for a user to dedupe their own file set
> > unless they do it as root, or ensure that all files have write
> > permission. There's a couple of duperemove bugs open for this:
> >
> > https://github.com/markfasheh/duperemove/issues/129
> > https://github.com/markfasheh/duperemove/issues/86
> >
> > The other problem we have is also related to forcing the user to open
> > target files for write - A process trying to exec a file currently
> > being deduped gets ETXTBUSY. The answer (as above) is to allow them to
> > open the targets ro - root can already do this. There was a patch from
> > Adam Borowski to fix this back in 2016:
> >
> > https://lkml.org/lkml/2016/7/17/130
> >
> > which I have incorporated into my changes.
> >
> >
> > The 2nd patch fixes our return code for permission denied to be
> > EPERM. For some reason we're returning EINVAL - I think that's
> > probably my fault. At any rate, we need to be returning something
> > descriptive of the actual problem, otherwise callers see EINVAL and
> > can't really make a valid determination of what's gone wrong.
> >
> > This has also popped up in duperemove, mostly in the form of cryptic
> > error messages. Because this is a code returned to userspace, I did
> > check the other users of extent-same that I could find. Both 'bees'
> > and 'rust-btrfs' do the same as duperemove and simply report the error
> > (as they should).
> >
> > Please apply.
> >
> > Thanks,
> > --Mark
> >
> > Changes from V3 to V4:
> > - Add a patch (below) to ioctl_fideduperange.2 explaining our
> > changes. I will send this patch once the kernel update is
> > accepted. Thanks to Darrick Wong for this suggestion.
> > - V3 discussion: https://www.spinics.net/lists/linux-btrfs/msg79135.html
> >
> > Changes from V2 to V3:
> > - Return bool from allow_file_dedupe
> > - V2 discussion: https://www.spinics.net/lists/linux-btrfs/msg78421.html
> >
> > Changes from V1 to V2:
> > - Add inode_permission check as suggested by Adam Borowski
> > - V1 discussion: https://marc.info/?l=linux-xfs&m=152606684017965&w=2
> >
> >
> > From: Mark Fasheh <mfasheh@suse.de>
> >
> > [PATCH] ioctl_fideduperange.2: clarify permission requirements
> >
> > dedupe permission checks were recently relaxed - update our man page to
> > reflect those changes.
> >
> > Signed-off-by: Mark Fasheh <mfasheh@suse.de>
> > ---
> > man2/ioctl_fideduperange.2 | 8 +++++---
>
> Mmm, man page update, thank you for editing the documentation too!
>
> Please cc linux-api and Michael Kerrisk so this can go upstream.
From: Mark Fasheh <mfasheh@suse.de>
[PATCH] ioctl_fideduperange.2: clarify permission requirements
dedupe permission checks were recently relaxed - update our man page to
reflect those changes.
Signed-off-by: Mark Fasheh <mfasheh@suse.de>
---
man2/ioctl_fideduperange.2 | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/man2/ioctl_fideduperange.2 b/man2/ioctl_fideduperange.2
index 84d20a276..4040ee064 100644
--- a/man2/ioctl_fideduperange.2
+++ b/man2/ioctl_fideduperange.2
@@ -105,9 +105,12 @@ The field
must be zero.
During the call,
.IR src_fd
-must be open for reading and
+must be open for reading.
.IR dest_fd
-must be open for writing.
+can be open for writing, or reading.
+If
+.IR dest_fd
+is open for reading, the user must have write access to the file.
The combined size of the struct
.IR file_dedupe_range
and the struct
@@ -185,8 +188,8 @@ This can appear if the filesystem does not support deduplicating either file
descriptor, or if either file descriptor refers to special inodes.
.TP
.B EPERM
-.IR dest_fd
-is immutable.
+This will be returned if the user lacks permission to dedupe the file referenced by
+.IR dest_fd .
.TP
.B ETXTBSY
One of the files is a swap file.
--
2.15.1
^ permalink raw reply related
* Re: [PATCH v2 5/7] mm: rename and change semantics of nr_indirectly_reclaimable_bytes
From: Vlastimil Babka @ 2018-07-17 19:11 UTC (permalink / raw)
To: Roman Gushchin
Cc: Andrew Morton, linux-mm, linux-kernel, Michal Hocko,
Johannes Weiner, linux-api, Christoph Lameter, David Rientjes,
Mel Gorman, Matthew Wilcox, Vijayanand Jitta, Laura Abbott,
Sumit Semwal
In-Reply-To: <20180717185451.GA18762@castle.DHCP.thefacebook.com>
On 07/17/2018 08:54 PM, Roman Gushchin wrote:
> On Tue, Jul 17, 2018 at 10:44:07AM +0200, Vlastimil Babka wrote:
>> On 07/02/2018 06:52 PM, Roman Gushchin wrote:
>>> On Sat, Jun 30, 2018 at 12:09:27PM +0200, Vlastimil Babka wrote:
>>>
>>> If these per-cpu data is something like per-cpu refcounters,
>>> which are using to manage reclaimable objects (e.g. cgroup css objects).
>>> Of course, they are not always reclaimable, but in certain states.
>>
>> BTW, seems you seem interested, could you provide some more formal
>> review as well? Others too. We don't need to cover all use cases
>> immediately, when the patchset is apparently stalled due to lack of
>> review. Thanks!
>
> Sure!
Thanks!
> The patchset looks sane at a first glance, but I need some time
> to dig deeper. Is v2 the final version?
There was a fixlet on top and some added changelog text, so I'll do a v3
tomorrow incorporating that to make things easier for everyone.
> Thanks!
>
^ permalink raw reply
* Re: [PATCH v2 5/7] mm: rename and change semantics of nr_indirectly_reclaimable_bytes
From: Roman Gushchin @ 2018-07-17 18:54 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Andrew Morton, linux-mm, linux-kernel, Michal Hocko,
Johannes Weiner, linux-api, Christoph Lameter, David Rientjes,
Mel Gorman, Matthew Wilcox, Vijayanand Jitta, Laura Abbott,
Sumit Semwal
In-Reply-To: <bfdb3fb1-5d81-e17c-e456-083cca04e2cc@suse.cz>
On Tue, Jul 17, 2018 at 10:44:07AM +0200, Vlastimil Babka wrote:
> On 07/02/2018 06:52 PM, Roman Gushchin wrote:
> > On Sat, Jun 30, 2018 at 12:09:27PM +0200, Vlastimil Babka wrote:
> >> On 06/29/2018 11:12 PM, Roman Gushchin wrote:
> >>>>
> >>>> The vmstat counter NR_INDIRECTLY_RECLAIMABLE_BYTES was introduced by commit
> >>>> eb59254608bc ("mm: introduce NR_INDIRECTLY_RECLAIMABLE_BYTES") with the goal of
> >>>> accounting objects that can be reclaimed, but cannot be allocated via a
> >>>> SLAB_RECLAIM_ACCOUNT cache. This is now possible via kmalloc() with
> >>>> __GFP_RECLAIMABLE flag, and the dcache external names user is converted.
> >>>>
> >>>> The counter is however still useful for accounting direct page allocations
> >>>> (i.e. not slab) with a shrinker, such as the ION page pool. So keep it, and:
> >>>
> >>> Btw, it looks like I've another example of usefulness of this counter:
> >>> dynamic per-cpu data.
> >>
> >> Hmm, but are those reclaimable? Most likely not in general? Do you have
> >> examples that are?
> >
> > If these per-cpu data is something like per-cpu refcounters,
> > which are using to manage reclaimable objects (e.g. cgroup css objects).
> > Of course, they are not always reclaimable, but in certain states.
>
> BTW, seems you seem interested, could you provide some more formal
> review as well? Others too. We don't need to cover all use cases
> immediately, when the patchset is apparently stalled due to lack of
> review. Thanks!
Sure!
The patchset looks sane at a first glance, but I need some time
to dig deeper. Is v2 the final version?
Thanks!
^ permalink raw reply
* Re: [PATCH] [RFC] y2038: globally rename compat_time to old_time32
From: Arnd Bergmann @ 2018-07-17 16:16 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Deepa Dinamani, y2038 Mailman List, Thomas Gleixner,
Geert Uytterhoeven, Linux Kernel Mailing List, Linux ARM,
Linux FS-devel Mailing List, linux-xfs, Linux API, alsa-devel
In-Reply-To: <20180717125855.GA3681@infradead.org>
On Tue, Jul 17, 2018 at 2:58 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Jul 16, 2018 at 12:11:53PM +0200, Arnd Bergmann wrote:
>> I don't think it makes a big difference whether we change it now or later,
>> but if Christoph feels that it addresses his concern about the compat_
>> namespace being reused during the transition, doing it earlier would
>> enable us to finish the remaining syscalls.
>
> Yes, I've been looking at your new series and the reuse of compat_*
> is really confusing
Ok, let's do that then. Do you think it's sufficient to change it on top
of the existing patch series? While I can rebase everything on top of
the rename, the header file dependencies are a bit tricky and adding
a patch at the end would make it easier to ensure that I don't
introduce any bisection issues.
Arnd
^ permalink raw reply
* Re: [PATCH v2 06/17] y2038: Change sys_utimensat() to use __kernel_timespec
From: Arnd Bergmann @ 2018-07-17 14:27 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Thomas Gleixner, y2038 Mailman List, Linux API, linux-arch,
GNU C Library, Albert ARIBAUD, Networking, Al Viro,
Peter Zijlstra, Darren Hart, Eric W . Biederman,
Dominik Brodowski
In-Reply-To: <20180717125243.GE25416@infradead.org>
On Tue, Jul 17, 2018 at 2:52 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Jul 16, 2018 at 06:10:52PM +0200, Arnd Bergmann wrote:
>> When 32-bit architectures get changed to support 64-bit time_t,
>> utimensat() needs to use the new __kernel_timespec structure as its
>> argument.
>>
>> The older utime(), utimes() and futimesat() system calls don't need a
>> corresponding change as they are no longer used on C libraries that have
>> 64-bit time support.
>>
>> As we do for the other syscalls that have timespec arguments, we reuse
>> the 'compat' syscall entry points to implement the traditional four
>> interfaces, and only leave the new utimensat() as a native handler,
>> so that the same code gets used on both 32-bit and 64-bit kernels
>> on each syscall.
>
> I wonder about the direction here: wouldn't it be easier to just
> leave th existing syscall names as-is and introduce a new utimesat64
> which uses the new timespec? We can then drop the old legacy utimesat
> for new architectures added after the cutover.
We have debated both approaches over several years, but for
most syscalls, we picked the approach described above, for multiple
reasons:
- For any system call that takes a time_t derived argument, we
already have two implementations (native and compat), so adding a
third one requires duplicating some code.
- I want to avoid adding an implementation that is not well tested
if possible, to make it less likely to introduce security holes or
subtle bugs that we can't fix later without breaking the ABI.
Using the same implementation for the new 32-bit case that
we have for the existing 64-bit case means that this code is
much better exercised, while reusing the compat code for the
traditional native syscall means it gets exercised by all current
user space, which makes it more likely to catch bugs early.
- Looking at the end result, I find it more logical to have each
of the converted syscalls implement the same binary interface
on both 32-bit and 64-bit architectures with the same code,
and have the old 32-bit implementation be similarly shared.
This is even more important once we add new architectures
that don't even provide the 32-bit time_t interfaces and just
leave out the old entry points.
Arnd
^ permalink raw reply
* Re: [PATCH v2 02/17] y2038: Remove newstat family from default syscall set
From: Arnd Bergmann @ 2018-07-17 14:18 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Thomas Gleixner, y2038 Mailman List, Linux API, linux-arch,
GNU C Library, Albert ARIBAUD, Networking, Al Viro,
Peter Zijlstra, Darren Hart, Eric W . Biederman,
Dominik Brodowski
In-Reply-To: <20180717125039.GB25416@infradead.org>
On Tue, Jul 17, 2018 at 2:50 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Jul 16, 2018 at 06:10:48PM +0200, Arnd Bergmann wrote:
>> We have four generations of stat() syscalls:
>> - the oldstat syscalls that are only used on the older architectures
>> - the newstat family that is used on all 64-bit architectures but
>> lacked support for large files on 32-bit architectures.
>> - the stat64 family that is used mostly on 32-bit architectures to
>> replace newstat
>> - statx() to replace all of the above, adding 64-bit timestamps among
>> other things.
>>
>> We already compile stat64 only on those architectures that need it,
>> but newstat is always built, including on those that don't reference
>> it. This adds a new __ARCH_WANT_NEW_STAT symbol along the lines of
>> __ARCH_WANT_OLD_STAT and __ARCH_WANT_STAT64 to control compilation of
>> newstat. All architectures that need it use an explict define, the
>> others now get a little bit smaller, and future architecture (including
>> 64-bit targets) won't ever see it.
>>
>> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Do I read this right that you only want to provide statx by default?
Yes, that is correct.
> It is a little different from the traditional stat calls, so I'd like
> to know this is actually ok from libc folks first.
That would definitely help. See below for the stat implementation
I did in my musl libc prototype based on statx(). It passes the
LTP syscall tests, but that doesn't mean all the corner cases
are correct.
Arnd
diff --git a/src/stat/fstat.c b/src/stat/fstat.c
index ab4afc0..b4f2027 100644
--- a/src/stat/fstat.c
+++ b/src/stat/fstat.c
@@ -1,3 +1,4 @@
+#define _BSD_SOURCE
#include <sys/stat.h>
#include <errno.h>
#include <fcntl.h>
@@ -8,6 +9,9 @@ void __procfdname(char *, unsigned);
int fstat(int fd, struct stat *st)
{
+#ifdef __USE_TIME_BITS64
+ return __statx(fd, "", st, AT_EMPTY_PATH | AT_STATX_SYNC_AS_STAT);
+#else
int ret = __syscall(SYS_fstat, fd, st);
if (ret != -EBADF || __syscall(SYS_fcntl, fd, F_GETFD) < 0)
return __syscall_ret(ret);
@@ -19,6 +23,7 @@ int fstat(int fd, struct stat *st)
#else
return syscall(SYS_fstatat, AT_FDCWD, buf, st, 0);
#endif
+#endif
}
LFS64(fstat);
diff --git a/src/stat/fstatat.c b/src/stat/fstatat.c
index 863d526..80add64 100644
--- a/src/stat/fstatat.c
+++ b/src/stat/fstatat.c
@@ -1,10 +1,53 @@
+#define _BSD_SOURCE
#include <sys/stat.h>
+#include <statx.h>
+#include <fcntl.h>
+#include <errno.h>
#include "syscall.h"
#include "libc.h"
+#ifdef __USE_TIME_BITS64
+
+#define AT_FLAGS (AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT | AT_EMPTY_PATH)
+
+int __statx(int fd, const char *restrict path, struct stat *restrict
buf, int flag)
+{
+ struct statx stx;
+ int ret;
+
+ ret = syscall(SYS_statx, fd, path, flag, STATX_BASIC_STATS, &stx);
+
+ buf->st_dev = (dev_t)stx.stx_dev_major << 32 | stx.stx_dev_minor;
+ buf->st_ino = stx.stx_ino;
+ buf->st_mode = stx.stx_mode;
+ buf->st_nlink = stx.stx_nlink;
+ buf->st_uid = stx.stx_uid;
+ buf->st_gid = stx.stx_gid;
+ buf->st_rdev = (dev_t)stx.stx_rdev_major << 32 | stx.stx_rdev_minor;
+ buf->st_size = stx.stx_size;
+ buf->st_blksize = stx.stx_blksize;
+ buf->st_blocks = stx.stx_blocks;
+ buf->st_atim.tv_sec = stx.stx_atime.tv_sec;
+ buf->st_mtim.tv_sec = stx.stx_atime.tv_sec;
+ buf->st_ctim.tv_sec = stx.stx_atime.tv_sec;
+ buf->st_atim.tv_nsec = stx.stx_atime.tv_nsec;
+ buf->st_mtim.tv_nsec = stx.stx_atime.tv_nsec;
+ buf->st_ctim.tv_nsec = stx.stx_atime.tv_nsec;
+
+ return ret;
+}
+#endif
+
int fstatat(int fd, const char *restrict path, struct stat *restrict
buf, int flag)
{
+#ifdef __USE_TIME_BITS64
+ if (flag & ~AT_FLAGS)
+ return __syscall_ret(-EINVAL);
+
+ return __statx(fd, path, buf, flag);
+#else
return syscall(SYS_fstatat, fd, path, buf, flag);
+#endif
}
LFS64(fstatat);
diff --git a/src/stat/lstat.c b/src/stat/lstat.c
index 5e8b84f..ad2bddd 100644
--- a/src/stat/lstat.c
+++ b/src/stat/lstat.c
@@ -1,3 +1,4 @@
+#define _BSD_SOURCE
#include <sys/stat.h>
#include <fcntl.h>
#include "syscall.h"
@@ -5,7 +6,9 @@
int lstat(const char *restrict path, struct stat *restrict buf)
{
-#ifdef SYS_lstat
+#ifdef __USE_TIME_BITS64
+ return __statx(AT_FDCWD, path, buf, AT_SYMLINK_NOFOLLOW |
AT_STATX_SYNC_AS_STAT);
+#elif defined(SYS_lstat)
return syscall(SYS_lstat, path, buf);
#else
return syscall(SYS_fstatat, AT_FDCWD, path, buf, AT_SYMLINK_NOFOLLOW);
diff --git a/src/stat/stat.c b/src/stat/stat.c
index b4433a0..599903a 100644
--- a/src/stat/stat.c
+++ b/src/stat/stat.c
@@ -1,3 +1,4 @@
+#define _BSD_SOURCE
#include <sys/stat.h>
#include <fcntl.h>
#include "syscall.h"
@@ -5,7 +6,9 @@
int stat(const char *restrict path, struct stat *restrict buf)
{
-#ifdef SYS_stat
+#ifdef __USE_TIME_BITS64
+ return __statx(AT_FDCWD, path, buf, AT_STATX_SYNC_AS_STAT);
+#elif defined(SYS_stat)
return syscall(SYS_stat, path, buf);
#else
return syscall(SYS_fstatat, AT_FDCWD, path, buf, 0);
^ permalink raw reply related
* Re: [PATCH] [RFC] y2038: globally rename compat_time to old_time32
From: Christoph Hellwig @ 2018-07-17 12:58 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Deepa Dinamani, y2038 Mailman List, Thomas Gleixner,
Geert Uytterhoeven, Christoph Hellwig, Linux Kernel Mailing List,
Linux ARM, Linux FS-devel Mailing List, linux-xfs, Linux API,
alsa-devel
In-Reply-To: <CAK8P3a2xu4KGVM1BvTjtXKHfXpBQ2PMynAPybW4cRt7FB8ktwg@mail.gmail.com>
On Mon, Jul 16, 2018 at 12:11:53PM +0200, Arnd Bergmann wrote:
> I don't think it makes a big difference whether we change it now or later,
> but if Christoph feels that it addresses his concern about the compat_
> namespace being reused during the transition, doing it earlier would
> enable us to finish the remaining syscalls.
Yes, I've been looking at your new series and the reuse of compat_*
is really confusing
^ permalink raw reply
* Re: [PATCH v2 06/17] y2038: Change sys_utimensat() to use __kernel_timespec
From: Christoph Hellwig @ 2018-07-17 12:52 UTC (permalink / raw)
To: Arnd Bergmann
Cc: tglx, y2038, hch, linux-api, linux-arch, libc-alpha,
albert.aribaud, netdev, viro, peterz, dvhart, ebiederm, linux
In-Reply-To: <20180716161103.16239-7-arnd@arndb.de>
On Mon, Jul 16, 2018 at 06:10:52PM +0200, Arnd Bergmann wrote:
> When 32-bit architectures get changed to support 64-bit time_t,
> utimensat() needs to use the new __kernel_timespec structure as its
> argument.
>
> The older utime(), utimes() and futimesat() system calls don't need a
> corresponding change as they are no longer used on C libraries that have
> 64-bit time support.
>
> As we do for the other syscalls that have timespec arguments, we reuse
> the 'compat' syscall entry points to implement the traditional four
> interfaces, and only leave the new utimensat() as a native handler,
> so that the same code gets used on both 32-bit and 64-bit kernels
> on each syscall.
I wonder about the direction here: wouldn't it be easier to just
leave th existing syscall names as-is and introduce a new utimesat64
which uses the new timespec? We can then drop the old legacy utimesat
for new architectures added after the cutover.
^ permalink raw reply
* Re: [PATCH v2 05/17] asm-generic: Remove empty asm/unistd.h
From: Christoph Hellwig @ 2018-07-17 12:51 UTC (permalink / raw)
To: Arnd Bergmann
Cc: tglx, y2038, hch, linux-api, linux-arch, libc-alpha,
albert.aribaud, netdev, viro, peterz, dvhart, ebiederm, linux
In-Reply-To: <20180716161103.16239-6-arnd@arndb.de>
On Mon, Jul 16, 2018 at 06:10:51PM +0200, Arnd Bergmann wrote:
> Nothing is left in asm/unistd.h except for the redirect to
> uapi/asm/unistd.h, so removing the file simply leads to that one being
> used directly. The linux/export.h inclusion is a leftover from commit
> e1b5bb6d1236 ("consolidate cond_syscall and SYSCALL_ALIAS declarations")
> and should not be used anyway.
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH v2 04/17] asm-generic: Remove unneeded __ARCH_WANT_SYS_LLSEEK macro
From: Christoph Hellwig @ 2018-07-17 12:51 UTC (permalink / raw)
To: Arnd Bergmann
Cc: tglx, y2038, hch, linux-api, linux-arch, libc-alpha,
albert.aribaud, netdev, viro, peterz, dvhart, ebiederm, linux
In-Reply-To: <20180716161103.16239-5-arnd@arndb.de>
On Mon, Jul 16, 2018 at 06:10:50PM +0200, Arnd Bergmann wrote:
> The sys_llseek sytem call is needed on all 32-bit architectures and
> none of the 64-bit ones, so we can remove the __ARCH_WANT_SYS_LLSEEK guard
> and simplify the include/asm-generic/unistd.h header further.
>
> Since 32-bit tasks can run either natively or in compat mode on 64-bit
> architectures, we have to check for both !CONFIG_64BIT and CONFIG_COMPAT.
>
> There are a few 64-bit architectures that also reference sys_llseek
> in their 64-bit ABI (e.g. sparc), but I verified that those all
> select CONFIG_COMPAT, so the #if check is still correct here. It's
> a bit odd to include it in the syscall table though, as it's the
> same as sys_lseek() on 64-bit, but with strange calling conventions.
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH v2 02/17] y2038: Remove newstat family from default syscall set
From: Christoph Hellwig @ 2018-07-17 12:50 UTC (permalink / raw)
To: Arnd Bergmann
Cc: tglx, y2038, hch, linux-api, linux-arch, libc-alpha,
albert.aribaud, netdev, viro, peterz, dvhart, ebiederm, linux
In-Reply-To: <20180716161103.16239-3-arnd@arndb.de>
On Mon, Jul 16, 2018 at 06:10:48PM +0200, Arnd Bergmann wrote:
> We have four generations of stat() syscalls:
> - the oldstat syscalls that are only used on the older architectures
> - the newstat family that is used on all 64-bit architectures but
> lacked support for large files on 32-bit architectures.
> - the stat64 family that is used mostly on 32-bit architectures to
> replace newstat
> - statx() to replace all of the above, adding 64-bit timestamps among
> other things.
>
> We already compile stat64 only on those architectures that need it,
> but newstat is always built, including on those that don't reference
> it. This adds a new __ARCH_WANT_NEW_STAT symbol along the lines of
> __ARCH_WANT_OLD_STAT and __ARCH_WANT_STAT64 to control compilation of
> newstat. All architectures that need it use an explict define, the
> others now get a little bit smaller, and future architecture (including
> 64-bit targets) won't ever see it.
>
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Do I read this right that you only want to provide statx by default?
It is a little different from the traditional stat calls, so I'd like
to know this is actually ok from libc folks first.
^ permalink raw reply
* Re: [PATCH v2 01/17] y2038: compat: Move common compat types to asm-generic/compat.h
From: Christoph Hellwig @ 2018-07-17 12:49 UTC (permalink / raw)
To: Arnd Bergmann
Cc: tglx, y2038, hch, linux-api, linux-arch, libc-alpha,
albert.aribaud, netdev, viro, peterz, dvhart, ebiederm, linux
In-Reply-To: <20180716161103.16239-2-arnd@arndb.de>
On Mon, Jul 16, 2018 at 06:10:47PM +0200, Arnd Bergmann wrote:
> While converting compat system call handlers to work on 32-bit
> architectures, I found a number of types used in those handlers
> that are identical between all architectures.
>
> Let's move all the identical ones into asm-generic/compat.h to avoid
> having to add even more identical definitions of those types.
>
> For unknown reasons, mips defines __compat_gid32_t, __compat_uid32_t
> and compat_caddr_t as signed, while all others have them unsigned.
> This seems to be a mistake, but I'm leaving it alone here. The other
> types all differ by size or alignment on at least on architecture.
>
> compat_aio_context_t is currently defined in linux/compat.h but
> also needed for compat_sys_io_getevents(), so let's move it into
> the same place.
>
> While we still have not decided whether the 32-bit time handling
> will always use the compat syscalls, or in which form, I think this
> is a useful cleanup that we can merge regardless.
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 24/32] vfs: syscall: Add fsopen() to prepare for superblock creation [ver #9]
From: David Howells @ 2018-07-17 9:40 UTC (permalink / raw)
To: Andy Lutomirski
Cc: dhowells, Linus Torvalds, Andrew Lutomirski, Al Viro, Linux API,
linux-fsdevel, Linux Kernel Mailing List, Jann Horn,
Tycho Andersen
In-Reply-To: <F80F34FD-CFD3-4A53-AECC-AEC8D9EF3EAE@amacapital.net>
Andy Lutomirski <luto@amacapital.net> wrote:
> > Whilst I'm at it, do we want the option of doing the equivalent of
> > mountat()? I.e. offering the option to open all the device files used by
> > a superblock with dfd and AT_* flags in combination with the filename?
> >
>
> Isn’t that more or less what I was suggesting? I suggested dfd and path and I also suggested just an fd and letting the caller open the file itself.
Do we need AT_* flags? There are three that we could use:
AT_SYMLINK_NOFOLLOW
AT_NO_AUTOMOUNT
AT_EMPTY_PATH
AT_EMPTY_PATH I can see, but I don't see it as likely that we'd want to use
the other two for selecting a source? Note that we can always do:
fsfd = fsopen("ext4");
sfd = open("/dev/", O_PATH);
fsconfig(fsfd, fsconfig_set_path, "journal_path", "sda1", sfd);
or:
fsfd = fsopen("ext4");
sfd = open("/dev/sda1", O_PATH);
fsconfig(fsfd, fsconfig_set_path_empty, "journal_path", "", sfd);
or:
fsfd = fsopen("ext4");
jfd = open("/dev/sda1", O_RDWR);
fsconfig(fsfd, fsconfig_set_fd, "journal_path", NULL, jfd);
assuming the open on the latter doesn't exclude the use by the filesystem.
This way I don't need a second syscall or a 6-arg syscall to handle path
specification.
David
^ permalink raw reply
* Re: [PATCH v2 5/7] mm: rename and change semantics of nr_indirectly_reclaimable_bytes
From: Vlastimil Babka @ 2018-07-17 8:44 UTC (permalink / raw)
To: Roman Gushchin
Cc: Andrew Morton, linux-mm, linux-kernel, Michal Hocko,
Johannes Weiner, linux-api, Christoph Lameter, David Rientjes,
Mel Gorman, Matthew Wilcox, Vijayanand Jitta, Laura Abbott,
Sumit Semwal
In-Reply-To: <20180702165223.GA17295@castle.DHCP.thefacebook.com>
On 07/02/2018 06:52 PM, Roman Gushchin wrote:
> On Sat, Jun 30, 2018 at 12:09:27PM +0200, Vlastimil Babka wrote:
>> On 06/29/2018 11:12 PM, Roman Gushchin wrote:
>>>>
>>>> The vmstat counter NR_INDIRECTLY_RECLAIMABLE_BYTES was introduced by commit
>>>> eb59254608bc ("mm: introduce NR_INDIRECTLY_RECLAIMABLE_BYTES") with the goal of
>>>> accounting objects that can be reclaimed, but cannot be allocated via a
>>>> SLAB_RECLAIM_ACCOUNT cache. This is now possible via kmalloc() with
>>>> __GFP_RECLAIMABLE flag, and the dcache external names user is converted.
>>>>
>>>> The counter is however still useful for accounting direct page allocations
>>>> (i.e. not slab) with a shrinker, such as the ION page pool. So keep it, and:
>>>
>>> Btw, it looks like I've another example of usefulness of this counter:
>>> dynamic per-cpu data.
>>
>> Hmm, but are those reclaimable? Most likely not in general? Do you have
>> examples that are?
>
> If these per-cpu data is something like per-cpu refcounters,
> which are using to manage reclaimable objects (e.g. cgroup css objects).
> Of course, they are not always reclaimable, but in certain states.
BTW, seems you seem interested, could you provide some more formal
review as well? Others too. We don't need to cover all use cases
immediately, when the patchset is apparently stalled due to lack of
review. Thanks!
> Thanks!
>
^ permalink raw reply
* Re: [PATCH v6] pidns: introduce syscall translate_pid
From: Nagarathnam Muthusamy @ 2018-07-16 17:57 UTC (permalink / raw)
To: Konstantin Khlebnikov, linux-api, linux-kernel, Jann Horn,
Serge Hallyn, Prakash Sangappa, Oleg Nesterov, Eric W. Biederman,
Andrew Morton, Andy Lutomirski, Michael Kerrisk (man-pages)
In-Reply-To: <152788068212.768348.15192457501079586650.stgit@buzz>
On 06/01/2018 12:18 PM, Konstantin Khlebnikov wrote:
> Each process have different pids, one for each pid namespace it belongs.
> When interaction happens within single pid-ns translation isn't required.
> More complicated scenarios needs special handling.
>
> For example:
> - reading pid-files or logs written inside container with pid namespace
> - writing logs with internal pids outside container for pushing them into
> - attaching with ptrace to tasks from different pid namespace
>
> Generally speaking, any cross pid-ns API with pids needs translation.
>
> Currently there are several interfaces that could be used here:
>
> Pid namespaces are identified by device and inode of /proc/[pid]/ns/pid.
>
> Pids for nested pid namespaces are shown in file /proc/[pid]/status.
> In some cases pid translation could be easily done using this information.
> Backward translation requires scanning all tasks and becomes really
> complicated for deeper namespace nesting.
>
> Unix socket automatically translates pid attached to SCM_CREDENTIALS.
> This requires CAP_SYS_ADMIN for sending arbitrary pids and entering
> into pid namespace, this expose process and could be insecure.
>
> This patch adds new syscall for converting pids between pid namespaces:
>
> pid_t translate_pid(pid_t pid, int source, int target);
>
> Pid-namespaces are referred file descriptors opened to proc files
> /proc/[pid]/ns/pid or /proc/[pid]/ns/pid_for_children.
> Negative argument points to current pid namespace.
>
> Syscall returns pid in target pid-ns or zero if task have no pid there.
>
> Error codes:
> EBADF - file descriptor is closed
> EINVAL - file descriptor isn't pid namespace
> ESRCH - task not found in @source namespace
>
> Translation could breach pid-ns isolation and return pids from outer pid
> namespaces iff process already has file descriptor for these namespaces.
>
> Examples:
> translate_pid(pid, ns, -1) - get pid in our pid namespace
> translate_pid(pid, -1, ns) - get pid in other pid namespace
> translate_pid(1, ns, -1) - get pid of init task for namespace
> translate_pid(pid, -1, ns) > 0 - is pid is reachable from ns?
> translate_pid(1, ns1, ns2) > 0 - is ns1 inside ns2?
> translate_pid(1, ns1, ns2) == 0 - is ns1 outside ns2?
> translate_pid(1, ns1, ns2) == 1 - is ns1 equal ns2?
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> Reanimated-by: Nagarathnam Muthusamy <nagarathnam.muthusamy@oracle.com>
>
> ---
>
> v1: https://lkml.org/lkml/2015/9/15/411
> v2: https://lkml.org/lkml/2015/9/24/278
> * use namespace-fd as second/third argument
> * add -pid for getting parent pid
> * move code into kernel/sys.c next to getppid
> * drop ifdef CONFIG_PID_NS
> * add generic syscall
> v3: https://lkml.org/lkml/2015/9/28/3
> * use proc_ns_fdget()
> * update description
> * rebase to next-20150925
> * fix conflict with mlock2
> v4: https://lkml.org/lkml/2017/10/13/177
> * rename from getvpid() into translate_pid()
> * remove syscall if CONFIG_PID_NS=n
> * drop -pid for parent task
> * drop fget-fdget optimizations
> * add helper get_pid_ns_by_fd()
> * wire only into x86
> v5: https://lkml.org/lkml/2018/4/4/677
> * rewrite commit message
> * resolve pidns by task pid or by pidns fd
> * add arguments source_type and target_type
> v6:
> * revert back minimized v4 design
> * rebase to next-20180601
> * fix COND_SYSCALL stub
> * use next syscall number, old used for io_pgetevents
>
> --- sample tool ---
>
> #define _GNU_SOURCE
> #include <sys/syscall.h>
> #include <sys/types.h>
> #include <fcntl.h>
> #include <unistd.h>
> #include <stdlib.h>
> #include <stdio.h>
> #include <err.h>
>
> #ifndef SYS_translate_pid
> #ifdef __x86_64__
> #define SYS_translate_pid 334
> #elif defined __i386__
> #define SYS_translate_pid 386
> #endif
> #endif
>
> pid_t translate_pid(pid_t pid, int source, int target) {
> return syscall(SYS_translate_pid, pid, source, target);
> }
>
> int main(int argc, char **argv) {
> int pid, source, target;
> char buf[64];
>
> if (argc != 4)
> errx(1, "usage: %s <pid> <source> <target>", argv[0]);
>
> pid = atoi(argv[1]);
> source = atoi(argv[2]);
> target = atoi(argv[3]);
>
> if (source > 0) {
> snprintf(buf, sizeof(buf), "/proc/%d/ns/pid", source);
> source = open(buf, O_RDONLY);
> if (source < 0)
> err(2, "open source %s", buf);
> }
>
> if (target > 0) {
> snprintf(buf, sizeof(buf), "/proc/%d/ns/pid", target);
> target = open(buf, O_RDONLY);
> if (target < 0)
> err(2, "open target %s", buf);
> }
>
> pid = translate_pid(pid, source, target);
> if (pid < 0)
> err(2, "translate_pid");
>
> printf("%d\n", pid);
> return 0;
> }
>
> ---
> ---
> arch/x86/entry/syscalls/syscall_32.tbl | 1
> arch/x86/entry/syscalls/syscall_64.tbl | 1
> include/linux/syscalls.h | 1
> kernel/pid_namespace.c | 66 ++++++++++++++++++++++++++++++++
> kernel/sys_ni.c | 3 +
> 5 files changed, 72 insertions(+)
>
> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
> index 14a2f996e543..e70685750d43 100644
> --- a/arch/x86/entry/syscalls/syscall_32.tbl
> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
> @@ -397,3 +397,4 @@
> 383 i386 statx sys_statx __ia32_sys_statx
> 384 i386 arch_prctl sys_arch_prctl __ia32_compat_sys_arch_prctl
> 385 i386 io_pgetevents sys_io_pgetevents __ia32_compat_sys_io_pgetevents
> +386 i386 translate_pid sys_translate_pid __ia32_sys_translate_pid
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> index cd36232ab62f..ebfd89055424 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -342,6 +342,7 @@
> 331 common pkey_free __x64_sys_pkey_free
> 332 common statx __x64_sys_statx
> 333 common io_pgetevents __x64_sys_io_pgetevents
> +334 common translate_pid __x64_sys_translate_pid
>
> #
> # x32-specific system call numbers start at 512 to avoid cache impact
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 390e814fdc8d..3f33971cf1c8 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -843,6 +843,7 @@ asmlinkage long sys_clock_adjtime(clockid_t which_clock,
> struct timex __user *tx);
> asmlinkage long sys_syncfs(int fd);
> asmlinkage long sys_setns(int fd, int nstype);
> +asmlinkage long sys_translate_pid(pid_t pid, int source, int target);
> asmlinkage long sys_sendmmsg(int fd, struct mmsghdr __user *msg,
> unsigned int vlen, unsigned flags);
> asmlinkage long sys_process_vm_readv(pid_t pid,
> diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
> index 2a2ac53d8b8b..3b872cbbe264 100644
> --- a/kernel/pid_namespace.c
> +++ b/kernel/pid_namespace.c
> @@ -13,6 +13,7 @@
> #include <linux/user_namespace.h>
> #include <linux/syscalls.h>
> #include <linux/cred.h>
> +#include <linux/file.h>
> #include <linux/err.h>
> #include <linux/acct.h>
> #include <linux/slab.h>
> @@ -380,6 +381,71 @@ static void pidns_put(struct ns_common *ns)
> put_pid_ns(to_pid_ns(ns));
> }
>
> +static struct pid_namespace *get_pid_ns_by_fd(int fd)
> +{
> + struct pid_namespace *pidns;
> + struct ns_common *ns;
> + struct file *file;
> +
> + file = proc_ns_fget(fd);
> + if (IS_ERR(file))
> + return ERR_CAST(file);
> +
> + ns = get_proc_ns(file_inode(file));
> + if (ns->ops->type == CLONE_NEWPID)
> + pidns = get_pid_ns(to_pid_ns(ns));
> + else
> + pidns = ERR_PTR(-EINVAL);
> +
> + fput(file);
> + return pidns;
> +}
> +
> +/*
> + * translate_pid - convert pid in source pid-ns into target pid-ns.
> + * @pid: pid for translation
> + * @source: pid-ns file descriptor or -1 for active namespace
> + * @target: pid-ns file descriptor or -1 for active namesapce
> + *
> + * Returns pid in @target pid-ns, zero if task have no pid there,
> + * or -ESRCH if task with @pid does not found in @source pid-ns.
> + */
> +SYSCALL_DEFINE3(translate_pid, pid_t, pid, int, source, int, target)
> +{
> + struct pid_namespace *source_ns, *target_ns;
> + struct pid *struct_pid;
> + pid_t result;
> +
> + if (source >= 0) {
> + source_ns = get_pid_ns_by_fd(source);
> + result = PTR_ERR(source_ns);
> + if (IS_ERR(source_ns))
> + goto err_source;
> + } else
> + source_ns = task_active_pid_ns(current);
> +
> + if (target >= 0) {
> + target_ns = get_pid_ns_by_fd(target);
> + result = PTR_ERR(target_ns);
> + if (IS_ERR(target_ns))
> + goto err_target;
> + } else
> + target_ns = task_active_pid_ns(current);
> +
> + rcu_read_lock();
> + struct_pid = find_pid_ns(pid, source_ns);
> + result = struct_pid ? pid_nr_ns(struct_pid, target_ns) : -ESRCH;
> + rcu_read_unlock();
> +
> + if (target >= 0)
> + put_pid_ns(target_ns);
> +err_target:
> + if (source >= 0)
> + put_pid_ns(source_ns);
> +err_source:
> + return result;
> +}
> +
> static int pidns_install(struct nsproxy *nsproxy, struct ns_common *ns)
> {
> struct pid_namespace *active = task_active_pid_ns(current);
> diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
> index 06b4ccee0047..bf276e9ace9a 100644
> --- a/kernel/sys_ni.c
> +++ b/kernel/sys_ni.c
> @@ -153,6 +153,9 @@ COND_SYSCALL_COMPAT(kexec_load);
> COND_SYSCALL(init_module);
> COND_SYSCALL(delete_module);
>
> +/* kernel/pid_namespace.c */
> +COND_SYSCALL(translate_pid);
> +
> /* kernel/posix-timers.c */
>
> /* kernel/printk.c */
>
Ping :-) Checking back again for Ack or comments.
Thanks,
Nagarathnam.
^ permalink raw reply
* [PATCH v2 17/17] y2038: signal: Add compat_sys_rt_sigtimedwait_time64
From: Arnd Bergmann @ 2018-07-16 16:11 UTC (permalink / raw)
To: tglx
Cc: y2038, hch, linux-api, linux-arch, libc-alpha, albert.aribaud,
netdev, viro, peterz, dvhart, ebiederm, linux, Arnd Bergmann
In-Reply-To: <20180716161103.16239-1-arnd@arndb.de>
Now that 32-bit architectures have two variants of
sys_rt_sigtimedwaid() for 32-bit and 64-bit time_t, we also
need to have a second compat system call entry point on the
corresponding 64-bit architectures.
The traditional system call keeps getting handled
by compat_sys_rt_sigtimedwait(), and this adds a new
compat_sys_rt_sigtimedwait_time64() that differs only in
the timeout argument type.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
kernel/signal.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/kernel/signal.c b/kernel/signal.c
index 8d4382d5182f..332f019dc421 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3172,6 +3172,38 @@ COMPAT_SYSCALL_DEFINE4(rt_sigtimedwait, compat_sigset_t __user *, uthese,
}
#endif
+#ifdef CONFIG_COMPAT
+COMPAT_SYSCALL_DEFINE4(rt_sigtimedwait_time64, compat_sigset_t __user *, uthese,
+ struct compat_siginfo __user *, uinfo,
+ struct __kernel_timespec __user *, uts, compat_size_t, sigsetsize)
+{
+ sigset_t s;
+ struct timespec64 t;
+ siginfo_t info;
+ long ret;
+
+ if (sigsetsize != sizeof(sigset_t))
+ return -EINVAL;
+
+ if (get_compat_sigset(&s, uthese))
+ return -EFAULT;
+
+ if (uts) {
+ if (get_timespec64(&t, uts))
+ return -EFAULT;
+ }
+
+ ret = do_sigtimedwait(&s, &info, uts ? &t : NULL);
+
+ if (ret > 0 && uinfo) {
+ if (copy_siginfo_to_user32(uinfo, &info))
+ ret = -EFAULT;
+ }
+
+ return ret;
+}
+#endif
+
/**
* sys_kill - send a signal to a process
* @pid: the PID of the process
--
2.9.0
^ permalink raw reply related
* [PATCH v2 16/17] y2038: Make compat_sys_rt_sigtimedwait usable on 32-bit
From: Arnd Bergmann @ 2018-07-16 16:11 UTC (permalink / raw)
To: tglx
Cc: y2038, hch, linux-api, linux-arch, libc-alpha, albert.aribaud,
netdev, viro, peterz, dvhart, ebiederm, linux, Arnd Bergmann
In-Reply-To: <20180716161103.16239-1-arnd@arndb.de>
Once sys_rt_sigtimedwait() gets changed to a 64-bit time_t, we have
to provide compatibility support for existing binaries. Using the
compat_sys_rt_sigtimedwait() entry point is convenient because it allows
to share the implementation with 64-bit architectures.
Unfortunately, the get_compat_sigset() and copy_siginfo_to_user32()
functions are used in that function, but we can replace them with
trivial helpers that do the same thing as before.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
kernel/signal.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index 0418a499b9f3..8d4382d5182f 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3027,7 +3027,22 @@ int copy_siginfo_from_user32(struct siginfo *to,
}
return 0;
}
-#endif /* CONFIG_COMPAT */
+
+#else /* !CONFIG_COMPAT */
+
+/* 32-bit architectures only need to convert compat_time_t, not siginfo or sigset_t */
+
+#define compat_siginfo siginfo
+#define compat_sigset_t sigset_t
+#define copy_siginfo_to_user32 copy_siginfo_to_user
+static inline int get_compat_sigset(sigset_t *set, const sigset_t __user *compat)
+{
+ if (copy_from_user(set, compat, sizeof *set))
+ return -EFAULT;
+
+ return 0;
+}
+#endif /* !CONFIG_COMPAT */
/**
* do_sigtimedwait - wait for queued signals specified in @which
@@ -3125,7 +3140,7 @@ SYSCALL_DEFINE4(rt_sigtimedwait, const sigset_t __user *, uthese,
return ret;
}
-#ifdef CONFIG_COMPAT
+#ifdef CONFIG_COMPAT_32BIT_TIME
COMPAT_SYSCALL_DEFINE4(rt_sigtimedwait, compat_sigset_t __user *, uthese,
struct compat_siginfo __user *, uinfo,
struct compat_timespec __user *, uts, compat_size_t, sigsetsize)
--
2.9.0
^ permalink raw reply related
* [PATCH v2 15/17] y2038: signal: Change rt_sigtimedwait to use __kernel_timespec
From: Arnd Bergmann @ 2018-07-16 16:11 UTC (permalink / raw)
To: tglx
Cc: y2038, hch, linux-api, linux-arch, libc-alpha, albert.aribaud,
netdev, viro, peterz, dvhart, ebiederm, linux, Arnd Bergmann
In-Reply-To: <20180716161103.16239-1-arnd@arndb.de>
This changes sys_rt_sigtimedwait() to use get_timespec64(), changing
the timeout type to __kernel_timespec, which will be changed to use
a 64-bit time_t in the future. Since the do_sigtimedwait() core
function changes, we also have to modify the compat version of this
system call in the same way.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
include/linux/syscalls.h | 2 +-
kernel/signal.c | 17 +++++++++--------
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 6871f6901a70..9c1da8752724 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -635,7 +635,7 @@ asmlinkage long sys_rt_sigprocmask(int how, sigset_t __user *set,
asmlinkage long sys_rt_sigpending(sigset_t __user *set, size_t sigsetsize);
asmlinkage long sys_rt_sigtimedwait(const sigset_t __user *uthese,
siginfo_t __user *uinfo,
- const struct timespec __user *uts,
+ const struct __kernel_timespec __user *uts,
size_t sigsetsize);
asmlinkage long sys_rt_sigqueueinfo(pid_t pid, int sig, siginfo_t __user *uinfo);
diff --git a/kernel/signal.c b/kernel/signal.c
index 786bacc60649..0418a499b9f3 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3036,7 +3036,7 @@ int copy_siginfo_from_user32(struct siginfo *to,
* @ts: upper bound on process time suspension
*/
static int do_sigtimedwait(const sigset_t *which, siginfo_t *info,
- const struct timespec *ts)
+ const struct timespec64 *ts)
{
ktime_t *to = NULL, timeout = KTIME_MAX;
struct task_struct *tsk = current;
@@ -3044,9 +3044,9 @@ static int do_sigtimedwait(const sigset_t *which, siginfo_t *info,
int sig, ret = 0;
if (ts) {
- if (!timespec_valid(ts))
+ if (!timespec64_valid(ts))
return -EINVAL;
- timeout = timespec_to_ktime(*ts);
+ timeout = timespec64_to_ktime(*ts);
to = &timeout;
}
@@ -3094,11 +3094,12 @@ static int do_sigtimedwait(const sigset_t *which, siginfo_t *info,
* @sigsetsize: size of sigset_t type
*/
SYSCALL_DEFINE4(rt_sigtimedwait, const sigset_t __user *, uthese,
- siginfo_t __user *, uinfo, const struct timespec __user *, uts,
+ siginfo_t __user *, uinfo,
+ const struct __kernel_timespec __user *, uts,
size_t, sigsetsize)
{
sigset_t these;
- struct timespec ts;
+ struct timespec64 ts;
siginfo_t info;
int ret;
@@ -3110,7 +3111,7 @@ SYSCALL_DEFINE4(rt_sigtimedwait, const sigset_t __user *, uthese,
return -EFAULT;
if (uts) {
- if (copy_from_user(&ts, uts, sizeof(ts)))
+ if (get_timespec64(&ts, uts))
return -EFAULT;
}
@@ -3130,7 +3131,7 @@ COMPAT_SYSCALL_DEFINE4(rt_sigtimedwait, compat_sigset_t __user *, uthese,
struct compat_timespec __user *, uts, compat_size_t, sigsetsize)
{
sigset_t s;
- struct timespec t;
+ struct timespec64 t;
siginfo_t info;
long ret;
@@ -3141,7 +3142,7 @@ COMPAT_SYSCALL_DEFINE4(rt_sigtimedwait, compat_sigset_t __user *, uthese,
return -EFAULT;
if (uts) {
- if (compat_get_timespec(&t, uts))
+ if (compat_get_timespec64(&t, uts))
return -EFAULT;
}
--
2.9.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox