On Tue, Aug 18, 2020 at 03:23:09PM -0700, Yonghong Song wrote:
>
> We did not use cond_resched() since for some iterators, e.g.,
> netlink iterator, where rcu read_lock critical section spans between
> consecutive seq_ops->next(), which makes impossible to do cond_resched()
> in the key while loop of function bpf_seq_read().
but after this patch we can, right?
>
> +/* maximum visited objects before bailing out */
> +#define MAX_ITER_OBJECTS 1000000
> +
> /* bpf_seq_read, a customized and simpler version for bpf iterator.
> * no_llseek is assumed for this file.
> * The following are differences from seq_read():
> @@ -79,7 +82,7 @@ static ssize_t bpf_seq_read(struct file *file, char __user
> *buf, size_t size,
> {
> struct seq_file *seq = file->private_data;
> size_t n, offs, copied = 0;
> - int err = 0;
> + int err = 0, num_objs = 0;
> void *p;
>
> mutex_lock(&seq->lock);
> @@ -135,6 +138,7 @@ static ssize_t bpf_seq_read(struct file *file, char
> __user *buf, size_t size,
> while (1) {
> loff_t pos = seq->index;
>
> + num_objs++;
> offs = seq->count;
> p = seq->op->next(seq, p, &seq->index);
> if (pos == seq->index) {
> @@ -153,6 +157,15 @@ static ssize_t bpf_seq_read(struct file *file, char
> __user *buf, size_t size,
> if (seq->count >= size)
> break;
>
> + if (num_objs >= MAX_ITER_OBJECTS) {
> + if (offs == 0) {
> + err = -EAGAIN;
> + seq->op->stop(seq, p);
> + goto done;
> + }
> + break;
> + }
> +
should this block be after op->show() and error processing?
Otherwise bpf_iter_inc_seq_num() will be incorrectly incremented?
> err = seq->op->show(seq, p);
> if (err > 0) {
> bpf_iter_dec_seq_num(seq);
After op->stop() we can do cond_resched() in all cases,
since rhashtable walk does rcu_unlock in stop() callback, right?
I think copy_to_user() and mutex_unlock() don't do cond_resched()
equivalent work.
